-
Notifications
You must be signed in to change notification settings - Fork 70
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
Export lists of installed packages and validate on Travis #73
Conversation
There are actually quite a lot of packages (in addition to the known libicu52) missing from a build of cedar-14 in this repo, compared to the what's running on cedar-14 dynos after the rollback: This is puzzling, since the cedar-14 docker image uses Has the way that the production cedar-14 image is built been changed internally, and so no longer reflects the Dockerfile in this repo? |
The Cedar-14 stack image is still built using tools available in this repository. It executes debootstrap on each build. The result of that doesn't appear to be any different from the Docker image, though. I think the previous existence of FUSE is particularly suspicious. I'll try to dig into that with |
Overall, this change looks good. We held a retrospective on a stack image roll-out yesterday and a snapshot of installed packages is something we've wanted to do for a while. I'm wondering if we want to keep an entire graph in the source tree (but limiting to roots in the diff). That might be useful for later understanding how things change. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, I'm with @tt on this: it looks good, and this meets a need we've been debating internally. I'm for merging it, even if we end up refining the stored data down the line.
@@ -8,3 +8,9 @@ env: | |||
- STACK=cedar-14 | |||
script: | |||
- bin/docker-build.sh $STACK | |||
- | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a mild preference that this be written as a shell script in a file, rather than inline in .travis.yml
, as it makes the travis file a bit easier to read if the task has a name rather than just a bolus of code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree -- originally I was hoping it would be more of a one-liner but with trying to handle both the "dirty working directory" and "new untracked files" case that isn't possible with Git at present. Perhaps this can go in bin/runtests.sh
and in the future additional tests be added there (though admittedly failing tests if the working directory is dirty does seem weird locally..).
I'll hold off updating this PR until other PRs add whatever missing packages are deemed required (I'm presuming the whole of libgnome2-0
, libgnomevfs2-0
, libgconf2-4
, systemd
plus deps might be unnecessary?) .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would actually prefer that it stayed in the Travis file. It's only used there and I'd like to get rid of the top-level bin directory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@edmorley, unfortunately, some of those packages are used by heroku-buildpack-google-chrome and while it's unlikely that it's being used at runtime, it's an unnecessary risk.
Running This appears to be due to the Compare: That said, it's an extra package not a missing package, so orthogonal :-) To figure out the package dependency changes, I ran the following against both a cedar-14 dyno and the local docker image built from master: Output: Diffing these and focusing on packages that are present in both, but that have different "depends" or "recommends" ("suggests" aren't installed by default so are irrelevant here), found the following differences:
In the Docker image locally, running
After that, re-running the This is much closer to the dyno output, but there is now another package that can be seen to have changed its dependencies: systemd-services. It previously had a depends on Installing Running
Both of these are in It seems surprising that the dependencies would be allowed to change in a stable LTS release for a |
@edmorley, can you try rebasing this to trigger a new build? |
@tt done. I left the old package list (from existing dynos) in, so the remaining missing packages can be seen. Build log: |
The new list looks much better. Please update the checked in version and I'll merge this pull request. |
The docker-build.sh script now exports the list of all packages installed in the final images to the working directory. These package lists are checked-in, making any deviations both easy to spot locally plus allowing for their validation on Travis.
Done |
Thank you very much for this, @edmorley. |
You're welcome :-) |
The docker-build.sh script now exports the list of all packages installed in the final images to the working directory. These package lists are checked-in, making any deviations both easy to spot locally plus allowing for their validation on Travis.
As a one-off for this PR, I based
cedar-14/installed-packages.txt
not on the output fromdocker-build.sh
but instead on the output fromdpkg-query
run on a cedar-14 dyno (that is using the older image after the rollback, that has the correct packages installed).As such, the cedar-14 part of the Travis run should hopefully fail and in the diff show the regression that is about to be fixed in #71, demonstrating the check works.