more preventing of "apt clean" #208
Conversation
and use git's "-C" flag to chdir into a directory rather than pushd/popd
|
i wonder whether the but that's mostly a design choice, how independent modules must be (and whether the |
|
Thanks for fixing the square brackets for string comparison |
| @@ -85,7 +85,7 @@ fi | |||
| [ -n "$BASE_SSH_ENABLE" ] || BASE_SSH_ENABLE=yes | |||
|
|
|||
| #Store the commit used for CustomPiOS | |||
| [ -n "$BASE_COMMIT" ] || BASE_COMMIT=`pushd "${CUSTOM_PI_OS_PATH}" > /dev/null ; git rev-parse HEAD ; popd > /dev/null` | |||
There was a problem hiding this comment.
@umlaeute
This seems to break now for me, not sure why, investigating
There was a problem hiding this comment.
hmm. i obviously did not test in docker, though i do not understand why it would break.
the only possible breakage i can think of is, that the pushd/popd solution handles the case gracefully where ${CUSTOM_PI_OS_PATH} does not exist at all (in which case you could get the commitish of the current directory).
if this is indeed the desired behavior, then i think a somewhat easier to read solution would be
[ -n "$BASE_COMMIT" ] || BASE_COMMIT=$(cd "${CUSTOM_PI_OS_PATH}"; git rev-parse HEAD)(since the $() (like the ``) is executed in a sub-shell, there's no need to go back to the calling directory)
this would fail (in the case of a non-existing ${CUSTOM_PI_OS_PATH}) only if the the current directory isn't git-tracked. (though i think in this case the original code would fail as well)
otoh, the original $(pushd /bla; git rev-parse HEAD; popd) (that is: with an non-existing ${CUSTOM_PI_OS_PATH}) returns an error as well, so the script would fail if this dir did not exist.
🤔
There was a problem hiding this comment.
my reference to docker was triggered by #206 (comment) (for whatever reasons I assumed that the comment on is related to this regression)
There was a problem hiding this comment.
could you provide an example (or link to a repository) that shows the problem?
There was a problem hiding this comment.
@umlaeute The issue derives because the git repo is not available from the docker container. The solution would be to keep the commit that the container was built from is a agreed location. Then search for that, then for the git repo.
There was a problem hiding this comment.
i came to the same conclusion (but obviously forgot to comment).
and yes, i think that it should be part of the Docker buildout to embed such a frozen version information within the container (at least, if the BASE_COMMIT info is of any relevance with the Docker container; i guess mostly you would be interested in the commitish of the "consumer" OS, rather than the base CustomPiOS commitish)
While b47534c provides a
PKGUPGRADE_CLEANUPvariable to prevent thepkgupgrademodule from runningapt-get cleanin the cleanpu script, thebasemodule still unconditionally clears the apt-cache:CustomPiOS/src/modules/base/end_chroot_script
Lines 41 to 42 in 8a3a002
This PR introduces a new
BASE_APT_CLEANvariable, that can be set to anything but the defaultyesto preventapt-get cleanfrom being run.while I edited the files i couldn't resist the urge to drop some unnecessary bashisms and replace the oldstyle command substitution
``with the modern, stackable$()(but these changes are in separate commits, so feel free to ignore them)