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 Cargo.lock to generated files #93

Merged
merged 2 commits into from
Sep 16, 2024
Merged

Conversation

apljungquist
Copy link
Contributor

Without this, updating Cargo.lock in a way that is incompatible with the manifest files would not cause CI to fail, despite the use of the --locked option.

My theory is that when cargo-acap-build is invoked, directly from the Makefile or indirectly via cargo-acap-sdk, it uses cargo metadata which updates the lockfile, causing later cargo invocations to succeed even when using the --locked option.

Disproved theories include:

  • The cargo install commands in install-venv.sh update the lockfile (checking the diff immediately after this step reveals no diff)
  • Some other action, prior to make check_all update the lockfile (checking the diff immediately before this step reveals no diff)

Makefile:

  • Use cargo metadata because it reconciles the lockfile without updating packages as cargo update would, and faster and with fewer side effects than something like cargo build.
  • Force the target to make sure it the lockfile is updated and the check works as intended, even if run before or without the other checks.

Without this, updating `Cargo.lock` in a way that is incompatible with
the manifest files would not cause CI to fail, despite the use of the
`--locked` option.

My theory is that when `cargo-acap-build` is invoked, directly from the
`Makefile` or indirectly via `cargo-acap-sdk`, it uses `cargo metadata`
which updates the lockfile, causing later `cargo` invocations to
succeed even when using the `--locked` option.

Disproved theories include:
- The `cargo install` commands in `install-venv.sh` update the lockfile
  (checking the diff immediately after this step reveals no diff)
- Some other action, prior to `make check_all` update the lockfile
  (checking the diff immediately before this step reveals no diff)

`Makefile`:
- Use `cargo metadata` because it reconciles the lockfile without
  updating packages as `cargo update` would, and faster and with fewer
  side effects than something like `cargo build`.
- Force the target to make sure it the lockfile is updated and the
  check works as intended, even if run before or without the other
  checks.
@apljungquist apljungquist requested a review from a team as a code owner September 16, 2024 18:04
@apljungquist apljungquist merged commit 42cf92d into main Sep 16, 2024
2 checks passed
@apljungquist apljungquist deleted the augment_generated_files branch September 16, 2024 18:24
apljungquist added a commit that referenced this pull request Oct 9, 2024
This should have been done already in #62, but I forgot.

Also set the `--locked` option to improve reproducibility in theory;
In practice I have not observed any problems, not even when it was a
main suspect in #93.
apljungquist added a commit that referenced this pull request Oct 9, 2024
This should have been done already in #62, but I forgot.

Also set the `--locked` option to improve reproducibility in theory;
In practice I have not observed any problems, not even when it was a
main suspect in #93.
apljungquist added a commit that referenced this pull request Oct 10, 2024
* upstream/main:
  fix: Install `cargo-acap-sdk` in venv (#97)
  chore: release cargo-acap-sdk 0.2.0 (#95)
  chore(deps): bump dirs from 3.0.2 to 5.0.1 (#85)
  Add `Cargo.lock` to generated files (#93)
  chore: Build apps with custom profile (#90)
apljungquist added a commit that referenced this pull request Oct 15, 2024
…per_and_example

* upstream/main:
  Update rust toolchain to 1.80.1 (#100)
  fix!: Make builds reproducible in dev-container (#99)
  feat(axevent)!: Use generic callbacks and remove leak (#59)
  fix: Install `cargo-acap-sdk` in venv (#97)
  chore: release cargo-acap-sdk 0.2.0 (#95)
  chore(deps): bump dirs from 3.0.2 to 5.0.1 (#85)
  Add `Cargo.lock` to generated files (#93)
  chore: Build apps with custom profile (#90)
  Remove leftover `dbg!` and copy `.eap` to appropriate names (#91)
  Use `cargo` with `--locked` option in checks (#92)
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.

1 participant