-
Notifications
You must be signed in to change notification settings - Fork 2
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
Prevent APT using source lists from /etc/apt/sources.list.d/ #46
Conversation
The test failures on Travis are pre-existing (ie on I've tested this buildpack branch against a test app, and it successfully installed the packages that failed in the #45 example:
|
bin/compile
Outdated
@@ -54,7 +54,8 @@ else | |||
fi | |||
|
|||
APT_OPTIONS="-o debug::nolocking=true -o dir::cache=$APT_CACHE_DIR -o dir::state=$APT_STATE_DIR" | |||
APT_OPTIONS="$APT_OPTIONS -o dir::etc::sourcelist=$APT_SOURCES" | |||
# Override the use of /etc/apt/sources.list (sourcelist) and /etc/apt/sources.list.d/* (sourceparts). | |||
APT_OPTIONS="$APT_OPTIONS -o dir::etc::sourcelist=$APT_SOURCES -o dir::etc::sourceparts=-'" |
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.
Why have -'
as the value here? I would have expected this to be an empty or non-existent 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.
Oh sorry the trailing '
isn't meant to be there. The -
is since that's what several SO / GitHub code references use. I tried to find a more authoritative guide as to what the supported path scheme was, however the docs are pretty lacking.
I'd be open to trying an empty string here and seeing whether that works too.
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.
Trying with the empty string results in apt-get update
returning the man page (which doesn't cause the buildpack to fail early, due to #47).
I've switched it back to -
(minus the '
typo).
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.
So I found a reference to /dev/null
in the APT source, here:
https://salsa.debian.org/apt-team/apt/blob/f15e090301e0744471d0fbf86ea0d494f6c08215/apt-pkg/sourcelist.cc#L313
...so I've switched it one last time to use that instead now 😆
(And re-tested again to ensure that works)
4b0e477
to
6910e8c
Compare
Previously the buildpack only passed `-o dir::etc::sourcelist` to APT, which meant that APT still used the default `sourceparts` location of `/etc/apt/sources.list.d/`. This meant on cedar-14 this buildpack would use esm.ubuntu.com as an APT source (as of heroku/base-images/pull/140), which results in errors if the requested packages happened to have ESM-only updates available (since the ESM repository requires credentials since it's a paid Ubuntu offering). Fixes #45 / W-6224944.
6910e8c
to
0beae64
Compare
Since whilst `-` appears to work, `/dev/null` is what is referenced here: https://salsa.debian.org/apt-team/apt/blob/f15e090301e0744471d0fbf86ea0d494f6c08215/apt-pkg/sourcelist.cc#L313
And published:
|
Previously the buildpack only passed
-o dir::etc::sourcelist
to APT, which meant that APT still used the defaultsourceparts
location of/etc/apt/sources.list.d/
.This meant on cedar-14 this buildpack would use esm.ubuntu.com as an APT source (as of heroku/base-images/pull/140), which results in errors if the requested packages happened to have ESM-only updates available (since the ESM repository requires credentials since it's a paid Ubuntu offering).
Fixes #45 / W-6224944.