fossa unconditionally calls cargo generate-lockfile

[Swatinem]

As the title says, cargo generate-lockfile is being called unconditionally here:

fossa-cli/src/Strategy/Cargo.hs

Line 317 in bde67a0

_ <- context "Generating lockfile" $ errCtx (FailedToGenLockFile manifestFile) $ execThrow manifestDir cargoGenLockfileCmd

We have a project that already maintains a lockfile, and that lockfile has a specific git commit for a git dependency.

However generate-lockfile bumps that git commit to the branch HEAD, which then fails later on because that branch contains breaking changes. (more specifically, it removes a workspace crate that is directly depended upon)

[meghfossa]

Thank you for reporting this defect. I have created an internal ticket in our backlog to address this behavior. I or someone from the team will post an update to this thread once the patch/or/workaround lands.

[meghfossa]

@Swatinem are you able to provide cargo.toml file, I'm presuming in the project's manifest, git dependency is pinned with the branch reference?

For unblocking/workaround, I suggest rev to pin the git dependency in the manifest for now. This should block generate-lockfile command from bumping git commit to the branch's HEAD.

[dependencies]
regex = { git = "https://github.com/rust-lang/regex", rev = "711bf162ecccfe81c1032d9c01f5096b0a7b7c8b" }

[untitaker]

We (Sentry) have most likely observed this same bug for non-git dependencies as well, such as regex = "1.0". There the workaround is infeasible.

[skilly-lily]

The reason that cargo generate-lockfile is called unconditionally is because cargo metadata requires a lockfile and an up-to-date cargo index on the machine. In the case that either of those are missing, the cargo metadata call will (or at least used to, haven't confirmed that this behavior is still there) not only update the index, but download all of the dependency crates (which it doesn't necessarily need to do). In the case that the dependency crates have not been downloaded, this causes the cargo metadata call to take several minutes to complete, instead of < 2 seconds.

We do need to support operating in a way that doesn't update the lockfile, perhaps using --frozen or some other cargo tooling, but we will need to make sure that this "don't require dep crate downloads" functionality is preserved.

[skilly-lily]

@untitaker Can you explain why the workaround is infeasible there? If you pin the dependency in the Cargo.toml file, then you won't see any updates when cargo generate-lockfile is run. If you're not pinning the version, then the behavior is correct, as running cargo build would also update the lockfile. This was the main reason we chose to use cargo generate-lockfile: any updates to the lockfile are idempotent with running cargo build.

[untitaker]

@scruffystuffs

The workaround to pin by git SHA in toml definetly helps to reduce the amount of pending approvals required (considering that HEAD is updating quite often), but doesn't work in the general case.

declaring regex = "=1.0.0" doesn't scale with the amount of dependencies we have, does not play well by default with tooling such as cargo-edit and is not going to take care of transitive dependencies, so cargo build is not idempotent at all.

If you're not pinning the version, then the behavior is correct, as running cargo build would also update the lockfile.

The lockfile is checked in. It is not updated during cargo build, even if we don't pin dependencies in toml and don't use --frozen. Only when the version range in toml is conflicting with what's in the lockfile, cargo implicitly updates the lockfile on build.

[untitaker]

To give you some concrete examples, here are two repos where cargo generate-lockfile definetly won't create identical results even after pinning all versions in toml:

• https://github.com/getsentry/relay
• https://github.com/getsentry/symbolicator

The end result is that everytime a new version of any transitive dependency is released (not updated), it is seen as "new/changed dependency" by FOSSA on a commit that didn't change either file.

[skilly-lily]

Thanks for the explanation and examples. That will be very useful in testing. We're definitely going to fix this, we just need to try not to reintroduce the dep download footgun. As @meghfossa stated, we're tracking this internally, and we'll update this ticket when we've got a fix for this.

[untitaker]

@scruffystuffs i just checked locally, and I don't think cargo metadata refrains from downloading all crates even if the lockfile is freshly generated. To reproduce, check out Relay and:

rm -fr ~/.cargo/registry/  # make sure that package downloads are not reused from previous test
cargo generate-lockfile  # downloads the registry again
cargo metadata --offline  # will error out because it attempts to download all packages

Our fossa jobs in relay also take 2 minutes, so i believe those packages are indeed downloaded