matklad / cargo-xtask

840 stars 23 forks source link

Recommend using --locked, explain #19

Closed azdavis closed 2 years ago

azdavis commented 2 years ago

I realized that the act of running cargo xtask itself might generate or update a Cargo.lock file, since it ultimately is compiling and running rust code in the workspace. That seems possibly unintuitive/surprising, so I recommend we recommend using --locked in the alias.

Then, we know that (unless the contents of the xtask itself do so) running an xtask will not unexpectedly change our dependency manifest.

azdavis commented 2 years ago

Hm, I just realized after making this that if the repo ignores its Cargo.lock file (maybe it's a workspace of exclusively libraries), and if we're using xtask to build in ci etc, this'll probably not work.

Maybe we should recommend using --locked iff you check in your lockfile to version control. But then in that case, isn't the point of a lockfile to be such that, if it exists, it will not get updated unless you explicitly ask?

I might be confusing myself here.

azdavis commented 2 years ago

I guess what I was thinking is that the docs for cargo build (and run, etc) mention this about --locked:

If the lock file is missing, or it needs to be updated, Cargo will exit with an error.

Implying there are situations where the lockfile exists but still 'needs updating'.

I just think it would be weird to e.g. have this happen:

  1. have a repo whose cargo.lock is checked in
  2. trigger a ci build
  3. ci calls xtask to build, test, etc
  4. it turns out the current commit we're running the ci build for has a cargo.lock that needs updating
  5. the act of running cargo xtask to build, test, etc also silently first updates the cargo.lock, then goes and builds and tests the actual code with a now updated cargo.lock
epage commented 2 years ago

then goes and builds and tests the actual code with a now updated cargo.lock

Except those operations will also update the lock file unless steps are taken to ensure it isn't updated.

matklad commented 2 years ago

Yeah, I don't think we need this: cargo xtask updating lockfile if there are changes to Cargo.toml feels roughly equivalent to cargo build updating the lockfile?

azdavis commented 2 years ago

I guess the only time you'd want --locked is if you had some xtask that called cargo build etc, and those calls used --locked, and thus you had in your mental model that calling that xtask would never update the lockfile if it were out of date. But there's no way for the cargo alias to 'know' what the contents of an xtask are.

azdavis commented 2 years ago

Guess we can close this, thanks for the discussion.