-
Notifications
You must be signed in to change notification settings - Fork 32
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
Best practices for package-lock.json files on Travis #183
Comments
I like approach 3, it maintains the way that package-lock.json is meant to work and keeps its changes in version control. We already have bots committing changes to our repos as part of release processes, so there is precedent for this type of solution. |
Here's the (pretty outdated) yarn PR. I'm still sold that yarn w/ the offline cache is currently the best packaging solution for all our various use cases. Also, I'm a fan of the clean-cut switchover, which I don't think will be too disruptive or burdensome. Also, if cfpb/consumerfinance.gov#4482 is merged in, it will be even less disruptive, as yarn can be included in the docker image and not require an install for docker users. |
@wpears we'd have to update all the satellite apps to use yarn, wouldn't we? How trivial/non-trivial is that? |
Would the use of I'm not sure that it would resole the underlying issue that @chosak has flagged:
Here's the description of how it works:
|
We've moved to yarn and have yarn.lock files now. Perhaps this issue can be updated/closed? |
How should we handle Travis changing package-lock.json files when we build Python wheels there?
Our cfgov-refresh satellite packages are configured to use Travis to build installable Python wheel files when a new tag/release is created on GitHub. We use the Travis GitHub Releases feature for this.
The way we want the workflow to happen:
Importantly, we use the setuptools-git-version package to set the Python package version based on the git status. The way this works is that if your git status is clean and on a tag, the package version (and wheel file) will be versioned with that tag, e.g. on tag
1.0
it'd generate a wheel named something likepackage-1.0-py2-none-any.whl
. If your local git status has diverged from a tag, either by having additional commits or by having locally modified files the version will indicate as such, e.g.package-1.0.dev3+438a0f9-py2-none-any.whl
(the3
indicates the number of commits since the tag, and the438a0f9
is some kind of hash of the changes).If Travis were just checking out code at a tag and building a wheel, we'd expect the wheel name to always be clean and perfectly match the tag. You wouldn't expect the Travis build to either make any commits or "dirty" the local git environment by making file changes -- but it can, in the case of package-lock.json.
If the package-lock.json file as generated by the last developer who committed doesn't match the versions installed by Travis, the file will be modified locally. This can happen even if developers do everything right, due to differences between Mac and Linux (see npm/npm#17722 for details). This in turn causes a dirty git workspace, which causes incorrect Python wheel names.
We've implemented some workarounds for this on a few of our satellites, but I'm not sure what the right approach is. I'm opening this issue to solicit feedback from FEWDs and to reach a team consensus.
Approach 1: ignore package-lock.json
If we have Travis run
npm config set package-lock false
, npm will ignore the package-lock.json file completely. It won't read from it and won't update it, and will only use the package.json file. One place we do this now is on TDP. This "fixes" the dirty wheel name issue, as we're no longer modifying a local file, but it isn't perfect.The problem with this approach is that the Travis build might use versions that are very different from what developers use locally. It means that the package-lock.json file no longer accurately describes the versions of things used to build the released/deployed wheel file of the package. If package.json contains unconstrained versions (e.g.
"cf-core": "^4.2.0"
) then there's no restriction on what Travis might use (although, perhaps this is a problem with defining things that way).Approach 2: use package-lock.json but discard local changes
Instead of disabling use of package-lock.json, let Travis use it and modify it, but then discard it before building the Python wheel. This file shouldn't be getting included in the wheel anyway, so it matters less.
This might be better than the first approach because it's likely that Travis will use more of what is in package-lock.json, but there's still no guarantee that it would use all of what's in there. And, like the first approach, the package-lock.json still doesn't necessarily describe the exact versions that were used to build the released/deployed wheel.
Approach 3: let Travis update package-lock.json
One potential idea would be to let Travis maintain package-lock.json. On builds, it could make necessary changes and then commit them back to the repo. This would ensure that the version of package-lock.json in the repo always exactly matches what was used to build the release.
The ccdb5-ui repo currently has an implementation of this, although I've submitted a PR that removes this to make it work the same way as the rest of our satellites.
Which of these are least bad? I know that @wpears has suggested moving to yarn as an alternative to npm. Any other ideas to consider @cfpb/cfgov-frontends?
The text was updated successfully, but these errors were encountered: