Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add lock-files script #17

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tcharding
Copy link
Member

Add a script to create and update the minimal and recent lock files. On the way create a stdlib.sh script to reduce code duplication and also a template.sh to make writing the next script easier.

Note I could not get flag_verbose to work with sourcing the stdlib script without exporting a global variable which I didn't do so as not to pollute the environment.

If this merges we can remove the update-lock-files.sh script from other repos and use this new script.

@tcharding
Copy link
Member Author

There is a fair bit of shell here, I geeked out a bit, might not be seen as useful.

@tcharding
Copy link
Member Author

If this goes through my next task will be try to bring the generate-fuzz.sh logic to this repo.


# Make a best effort at creating a minimal lock file.
rm --force Cargo.lock
cargo +nightly check --all-features -Z minimal-versions
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In 65a74a1:

It seems like this script needs this line to fail in order to be useful. But I believe this fails on every single one of our repos becaue our minimal dependencies themselves don't specify their minimal dependencies correctly.

Though I guess I haven't tried it for some of the new leaf crates..

Copy link
Member Author

@tcharding tcharding Sep 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like this script needs this line to fail in order to be useful.

What do you mean by this? My main idea behind this script was that if this succeeds then win and one doesn't have to have the -Z command in shell history or in mind. And if it this fails the script documents my lazy 'just duplicate recent'. But since the script lives in an obscure repo is this even useful, I don't know?

I used -Z minimal-versions recently and it worked, to my surprise (maybe in bech32, but not sure).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used -Z minimal-versions recently and it worked, to my surprise (maybe in bech32, but not sure).

Ooo, nice.

I think the MSRV update to 1.63 solved a lot of our dependency compatibility issues.

And if it this fails the script documents my lazy 'just duplicate recent'

I don't think it's too useful as documentation because it's inside a script in a fairly obscure repo.

I also think we should move away from "duplicate the recent lockfile" to "just don't have a minimal lockfile". This will clearly signal which repos still need somebody to do the hard work of running -Z minimal then patching everything up.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess, if the script were to try using -Zminimal, and on failure, output a warning and not produce the lockfile, that'd be pretty cool.

I guess we also need to check that the "recent" lockfile produced by cargo update works with MSRV. Again, in my experience it almost never does. And we need to hard fail if neither lockfile can be produced.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good ideas, will re-spin.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently the script doesn't fail correctly in workspace because cargo check in a workspace does not exit with non-zero if the last crate builds successfully, I couldn't find a way to make it do so.

I also think we should move away from "duplicate the recent lockfile" to "just don't have a minimal lockfile".

Leaving this for another day.

Add a script to create and update the minimal and recent lock files. On
the way create a `stdlib.sh` script to reduce code duplication and also
a `template.sh` to make writing the next script easier.

Note I could not get `flag_verbose` to work with sourcing the `stdlib`
script without exporting a global variable which I didn't do so as not
to pollute the environment.

If this merges we can remove the `update-lock-files.sh` script from
other repos and use this new script.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants