xtask command to use a local registry

[bjoernQ]

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.

Cargo has a core assumption about source replacement that the source code is exactly the same from both sources. Note that this also means that a replacement source is not allowed to have crates which are not present in the original source.

This makes totally sense for regular use but I'd say in our case it's okay (and it a very temporary thing).

Since cargo is 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-tests hopefully explains things well enough)

We might want / need to add some more test projects under the compile-tests folder 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 )

[MabezDev]

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.

[bjoernQ]

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.

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 extern "C" fn malloc in esp-alloc won't break the test since the compiler is smart enough to not include the code which is actually using it in the linker step.

[SergioGasquez]

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!

[SergioGasquez]

Should we add this to .gitignore?

[bjoernQ]

I'm undecided what would cause more harm: unintentional commits or forgetting to commit something which should get committed

[bjoernQ]

oh sorry - that file is not changed but created just for the processing .... will add it to the ignore list

[bjoernQ]

My local testing suggests this is working so far. Probably the code could use some more love

Not sure if we want to automate things in a release in future but maybe we want to keep this isolated for now and give it a try during the next release first

[MabezDev]

How will we integrate this into our release workflow? I'm a bit concerned we're duplicated some release steps in a new relcheck command. Any reason not to make this part of the release process and use the same plan.json for creating the release environment?

[bjoernQ]

How will we integrate this into our release workflow? I'm a bit concerned we're duplicated some release steps in a new relcheck command. Any reason not to make this part of the release process and use the same plan.json for creating the release environment?

That is probably the what we want in the end - I hesitated to do a deeper integration before seeing this working fine manually with a real release. (That's also the reason this is as little integrated into anything as possible)

I didn't wanted to take the blame for stopping all engines during a release because something dumb is not quite right yet

(i.e. my idea was to do the manual verification after applying the plan and before publishing manually)

[MabezDev]

I didn't wanted to take the blame for stopping all engines during a release because something dumb is not quite right yet

I think that we'll want this as part of our release process asap. I don't think we particularly need to worry about blocking a release (after all we can always delay a release by a few days and no one will die).

I think I'd like to see this a bit more integrated before we land this. If we land this as a separate command, I fear it won't get used. If you're really worried about blocking the release, we can add a --skip-release-tests flag to one of the steps, or make the testing another "stage" of the release that we can optionally skip if we want to.

[bjoernQ]

I see - would you prefer to run this after applying the plan or before publishing? (as part of those commands)

There are a few very unfortunate things here - e.g. that whole priming of the local registry is not great (and I am thinking about ways to change that) - also modifying the manifests and then reverting it is ugly

Changing this to draft for now then - maybe the real solution will be a new PR or it will be this PR

[MabezDev]

would you prefer to run this after applying the plan or before publishing?

I think this should come between executing the plan (bumping all the versions and merging the PR) and publishing, as a final check before we actually publish. I need to formulate this into some documentation, but this is the flow of a release currently: #4732 (comment), I think it should be a step just after "from main with PR merged".

[MabezDev]

would you prefer to run this after applying the plan or before publishing?

I think this should come between executing the plan (bumping all the versions and merging the PR) and publishing, as a final check before we actually publish. I need to formulate this into some documentation, but this is the flow of a release currently: #4732 (comment), I think it should be a step just after "from main with PR merged".

On second thoughts, I'd like this step to be run in CI, not on the releaser's machine. So it probably makes sense to trigger this step as part of opening the bump PR (execution phase), we already do this with the docs (via label).

With all that in mind, maybe we need less changes than I thought here. I would just convert all the individual steps we have in the readme into a workflow that gets triggered by a label. The only things that might need to change here is that this PR won't need to bump packages, it it needs to do is seed the registry with the compile test application and then use the bumped versions from the release PR.

What do you think?

[bjoernQ]

Thanks!

Running it in CI sounds like a great idea - makes we less afraid of the temporarily modified files (need to look into the triggering of the docs workflow)

I probably need / want to change a few things to make that workflow simple and the general process more robust

[bjoernQ]

Before trying to create the workflow and polishing the code some instructions to test this.

• apply the attached patch - it will undo the changes for using the git deps for the PACs and patches the xtask's execute plan to not cry about a dirty repo and not performing any git actions
• (alternatively try to do it yourself until you are not using the PACs from git and things build)
• cargo xrelease plan --allow-non-main
• remove the comments from release_plan.jsonc
• cargo xrelease execute-plan --manual-pull-request --no-dry-run (make sure to have this patches as stated above)
• cargo xrel-check init
• cargo xrel-check update
• cargo xrel-check scrap-path-deps
• e.g. cargo xtask build examples all --chip=esp32s3
• e.g. cargo xtask build examples all --package=qa-test --chip=esp32s3
• e.g. cargo xtask build tests esp32s3 (fails for dedicated GPIO since we patched stuff to make things work this far)

You can repeat this and edit the release plan before executing it to see how a bad run would look like

diff.zip

[MabezDev]

I ran into an issue when following the steps above:

error: failed to sync

Caused by:
  failed to load manifest for workspace member `/home/mabez/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std`
  referenced by workspace at `/home/mabez/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/Cargo.toml`

Caused by:
  failed to parse manifest at `/home/mabez/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/Cargo.toml`

Caused by:
  feature `edition2024` is required

I'm using latest stable release here.

It's probably just some weirdness with my local setup. Overall having a brief look at the code, I think we're ready to try and integrate it into the workflow.

We might want to make the ci.yml workflow a composite one, then we can just invoke it here from the new workflow (we can also then do our nightly CI fully too).

[bjoernQ]

I added a workflow but I refrained from refactoring the workflows for now - turned out the overlap of ci.yml and the new workflow is quite small, also lets first see if this is a pile of crap needs some refinement before doing that? (Same for a few possible refactorings in the xtask)

Ofc the workflow currently runs into errors when trying to compile the examples/tests - we need a real release to test it

[MabezDev]

Overall this looks good to me! I would still like to eventually turn the CI workflow into a composite one and test here, but this is more than enough for a start. Thanks!