pub icon indicating copy to clipboard operation
pub copied to clipboard

feat: add `--set-exit-if-changed` on `pub upgrade`

Open alestiago opened this issue 1 year ago • 6 comments

Description

As a developer, I would like to have the option "--set-exit-if-changed" when running pub upgrade --tighten.

Semantics

From https://github.com/dart-lang/pub/issues/4146#issuecomment-1963885730:

I would expect --set-exit-if-changed to set the exit code on a change. See the following issue in the dart_style for reference https://github.com/dart-lang/dart_style/issues/365, or the implementation.

For example, if pub upgrade --tighten does not modify any file then the exit code should be 0. Otherwise, if a change was made the exit code should be non-0 (the one being used by dart_style is 1).

Use-case

From https://github.com/dart-lang/pub/issues/4146#issuecomment-1963898045:

The use-case is to be used in continuous integration (CI, for example GitHub workflows). I would like to easily check if a project has all its dependencies tighten up. Having it return a non-0 exit code will fail the workflow and disallow merging the change into the project until the requirement of "all dependencies are tighten up" is fulfilled.

One could parse dry-run to get the desired effect, but I see that as a "hacky" solution, since checking if the output of dry-run contains a specific string is fragile.

alestiago avatar Feb 26 '24 11:02 alestiago

What are the proposed semantics?

sigurdm avatar Feb 26 '24 11:02 sigurdm

I would expect --set-exit-if-changed to set the exit code on a change. See the following issue in the dart_style for reference https://github.com/dart-lang/dart_style/issues/365, or the implementation.

For example, if pub upgrade --tighten does not modify any file then the exit code should be 0. Otherwise, if a change was made the exit code should be non-0 (the one being used by dart_style is 1).

alestiago avatar Feb 26 '24 11:02 alestiago

What is the use-case?

sigurdm avatar Feb 26 '24 11:02 sigurdm

The use-case is to be used in continuous integration (CI, for example GitHub workflows). I would like to easily check if a project has all its dependencies tighten up. Having it return a non-0 exit code will fail the workflow and disallow merging the change into the project until the requirement of "all dependencies are tighten up" is fulfilled.

One could parse dry-run to get the desired effect, but I see that as a "hacky" solution, since checking if the output of dry-run contains a specific string is fragile.

alestiago avatar Feb 26 '24 11:02 alestiago

Perhaps this situation would be detectable in CI with git diff pubspec.yaml after dart pub upgrade --tighten

I'm not sure that is desirable in general to always have tightened dependencies... We would need to also have a way of opting out.

sigurdm avatar Feb 26 '24 11:02 sigurdm

Perhaps this situation would be detectable in CI with git diff pubspec.yaml after dart pub upgrade --tighten

That would also be a solution that avoids parsing the dry-run, thanks for sharing 🙌 . Although, I think it has some minor caveats:

  • It assumes that the name of the file pubspec.yaml -- (this is bearable)
  • It assumes that the only file that would only ever change is the pubspec.yaml -- (this is bearable)
  • Requires the git executable to be in the machine -- (possibly bearable)
  • If any CI steps before alters the pubspec.yaml it would require to commit the previous differences before ensuring tight dependencies. Note that some packages in the ecosystem use the pubspec.yaml to define/configure the package behaviour -- (possible bearable by isolating the tightening step if possible)
  • It would not be consistent with other commands the dartdev package has (where pub is being used), such as dart format.

Overall, I think adding --set-exit-if-changed would improve the developer experience.

I'm not sure that is desirable in general to always have tightened dependencies... We would need to also have a way of opting out.

I'm not sure if I'm understanding this concern correctly, I believe when the option --set-exit-if-changed is not specified the tool would have the same behaviour as the one we have today (opting out and always exiting with 0). It would be exactly the same as with dart_style.

alestiago avatar Feb 26 '24 11:02 alestiago