-
Notifications
You must be signed in to change notification settings - Fork 380
xtask command to use a local registry
#4655
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
base: main
Are you sure you want to change the base?
Conversation
|
This is really cool! Thanks for experimenting with this. Using local registries is a good idea, I forgot that I've already played with them this year with those semver experiments :D. I need to play around with this a bit before I start reviewing but I have some initial thoughts. Can we reuse our current test suite and examples? We have great coverage already, it would be great to reuse these in the "release test" instead of separate compile tests. Note that I'm happy for this to land with separate tests with a view to reusing the tests later. |
Probably yes - after some pre-processing we should be able to check with the "upcoming" release - maybe in addition to the (hopefully) zero-maintenance compile-tests. I already just identified one thing where the compile-tests idea is lacking: e.g. renaming |
7542fb6 to
23f4f61
Compare
SergioGasquez
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was able to run the tests on my machine, the README was very useful and made things very easy! Thanks! Left a few comments/nitpicks!
| let _ = std::fs::create_dir("compile-tests/.cargo"); | ||
|
|
||
| std::fs::write( | ||
| "compile-tests/.cargo/config.toml", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add this to .gitignore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm undecided what would cause more harm: unintentional commits or forgetting to commit something which should get committed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh sorry - that file is not changed but created just for the processing .... will add it to the ignore list
23f4f61 to
9342fa6
Compare
This explores the idea of #4532
It's using a local registry (in the file system) instead of using a full-fledged registry. This makes things much more lightweight.
Instead of trying to "bend" things to use our registry for our crates only, this is using source replacement.
This makes things much easier and even more important: more realistic.
In theory this is violating one of the assumption Cargo has about it.
This makes totally sense for regular use but I'd say in our case it's okay (and it a very temporary thing).
Since
cargois aware of the replacement it won't mix up the registries in it's cache - the worst to happen might be ending up with some orphaned directories in the Cargo cache.This is not integrated into any of the release related tasks (yet) - it's just available as a set of xtask commands. But it already allows to estimate the consequences of updates. (the README.md in
compile-testshopefully explains things well enough)We might want / need to add some more test projects under the
compile-testsfolder to get a better coverage of different targets and feature sets. (But for the now this would just add noise - so I kept things simple)It would be good if this could get tested in different environments - on my side I tested it in Windows and WSL2.
Pretty sure there are still some rough edges we should flesh out before integrating this more.
(The code is POC-quality currently - I'm more interesting in someone else to try and play with it before polishing and nit-picking )