Skip to content

Add pubspec.lock to repository #749

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

Open
3 tasks done
mtrezza opened this issue May 21, 2022 · 9 comments
Open
3 tasks done

Add pubspec.lock to repository #749

mtrezza opened this issue May 21, 2022 · 9 comments
Labels
type:ci CI related issue

Comments

@mtrezza
Copy link
Member

mtrezza commented May 21, 2022

New Feature / Enhancement Checklist

Current Limitation

The pubspec.lock file is not checked in to this repository. This makes development more unstable and testing more volatile. While the lock file should not be published in the registry by convention, it may make sense to check it in for this repository.

Why the lock file is not supposed to be published in the registry:

  • Dependency resolution does not seem to be as sophisticated in dart as for example in npm. It can't handle if two dependencies require different versions of the same sub-dependency, see docs. It's even by design that an app developer overrides dependency ranges that a dependency developer specified for their package, which is truly bizarre and defies the purpose of package tests.
  • There seem to be no dependency upgrade / vulnerability notification services in the dart ecosystem (at least not that I now of), so locking in a specific version would create too much inertia and risk across the whole ecosystem. Allowing "packages to evolve independently" as they put it euphemistically is just a half-baked solution to overcome that. If a package fixes a vulnerability with a major version increment, the caret version range that is a key part of the dart dependency versioning convention doesn't help much and developers doesn't get any security notification anyway if new major version fixes a vulnerability.

Why it makes sense to check in the lock file for development:

The effect of the above shortcomings is that we never have a reproducible dependency tree. In a CI run, tests may pass in one job and fail in the next job if in the meantime a new version has been published with a bug. It makes collaboration on the SDK more difficult for the Parse Platform community, as N developers may use N different dependency trees when working on the same PR, and the GitHub CI tests may even use a N+1'th version of the dependency tree, each one using their own lock file.

Feature / Enhancement Description

  • Remove pubspec.lock from .gitignore so that it is checked in to this repository for development.
  • Add pubspec.lock to .pubignore so that it is not published in the registry as part of the package.

What we'd have to do manually is delete the lock file form time to time, e.g. with a new release and upgrade it in a controlled way. It can even be part of a PR where the developer regenerates the lock file. That way we have dependency consistency across our development pipeline.

Example Use Case

n/a

Alternatives / Workarounds

Live with the above mentioned risks, as seems to be accepted in dart ecosystem. Of the few pub.dev packages I've checked not a single one had their lock file in the GitHub repository.

@parse-github-assistant
Copy link

parse-github-assistant bot commented May 21, 2022

Thanks for opening this issue!

  • 🎉 We are excited about your ideas for improvement!

@mtrezza mtrezza added the type:ci CI related issue label May 21, 2022
@mtrezza mtrezza linked a pull request May 21, 2022 that will close this issue
2 tasks
@mtrezza mtrezza pinned this issue May 21, 2022
@fischerscode
Copy link
Contributor

Since it won't be published to pub.dev (I think regardless of .pubignore, but I'm not sure about that.) it's just a development thing.
From that perspective: +1 for your argumentation
An alternative for CI: dump pubspec.lock or upload it as an artifact if a test fails.

What we'd have to do manually is delete the lock file form time to time

flutter pub upgrade --major-versions might be our friend. We could even do some GitHub Actions magic.

@mtrezza
Copy link
Member Author

mtrezza commented May 23, 2022

Sounds good. I was surprised though that even popular dart packages do not have the lock file in their GitHub repo. So despite the arguments I am still hesitant to commit the lock file.

@mtrezza mtrezza unpinned this issue Nov 21, 2022
@mbfakourii
Copy link
Member

@mtrezza

Where did the issue go, should pubspec.lock be added?

@mtrezza
Copy link
Member Author

mtrezza commented May 18, 2023

This is still up for discussion.

@mbfakourii
Copy link
Member

It is generally recommended to include the pubspec.lock file in version control for development purposes. This ensures that all developers have a consistent set of dependencies and versions, which helps to avoid build and runtime errors caused by version conflicts. It also helps to ensure that the same dependencies and versions are used in continuous integration (CI) and deployment environments.

However, it is not recommended to include the pubspec.lock file in published packages, as this file contains specific version information that may not be applicable or compatible with other projects. Instead, when publishing a package, it is recommended to only include the pubspec.yaml file and let the package manager generate a new pubspec.lock file for each project that installs the package. This allows for more flexibility and compatibility with different dependency trees.

@mtrezza
Copy link
Member Author

mtrezza commented May 22, 2023

Makes sense, how can we not ignore it in GitHub, so that it's in the repo, but then ignore it for release so that it's not pushed to pub.dev?

@mbfakourii
Copy link
Member

I think we should use .pubignore, see here

@mtrezza
Copy link
Member Author

mtrezza commented May 22, 2023

Sounds good to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:ci CI related issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants