Skip to content

more preventing of "apt clean" #208

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.

Sign up for GitHub

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

Merged
guysoft merged 3 commits into from
Oct 1, 2023
Merged

more preventing of "apt clean" #208

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4629bc6
Add BASE_APT_CLEAN config to (prevent) cleanup of apt-cache
Sep 26, 2023
b71dbf8
modules/base: use POSIX string comparision
Sep 26, 2023
c9c3af5
modules/base: use $() instead of ``
Sep 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 11 additions & 7 deletions src/modules/base/config
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ BASE_VERSION=1.5.0
[ -n "$BASE_DISTRO" ] || BASE_DISTRO=raspbian

# Note: Set BASE_ZIP_IMG relative to the distro/src/workspace directory to pass a custom named file or an already extracted '.img'-file.
if [ "${BASE_DISTRO}" == "ubuntu" ]; then
if [ "${BASE_DISTRO}" = "ubuntu" ]; then
# Default image ubuntu
[ -n "$BASE_ZIP_IMG" ] || BASE_ZIP_IMG=`ls -t $BASE_IMAGE_PATH/ubuntu-*.xz | head -n 1`
[ -n "$BASE_ZIP_IMG" ] || BASE_ZIP_IMG=$(ls -t $BASE_IMAGE_PATH/ubuntu-*.xz | head -n 1)
# Default user ubuntu
[ -n "$BASE_USER" ] || BASE_USER=ubuntu

Expand All @@ -28,10 +28,10 @@ if [ "${BASE_DISTRO}" == "ubuntu" ]; then
[ -n "$BASE_USER_PASSWORD" ] || BASE_USER_PASSWORD=ubuntu
else
# Default image raspbian
if [ "${BASE_DISTRO}" == "raspios64" ]; then
[ -n "$BASE_ZIP_IMG" ] || BASE_ZIP_IMG=`ls -t $BASE_IMAGE_PATH/*-{raspbian,raspios}-*-arm64-*.{zip,7z,xz} | head -n 1`
if [ "${BASE_DISTRO}" = "raspios64" ]; then
[ -n "$BASE_ZIP_IMG" ] || BASE_ZIP_IMG=$(ls -t $BASE_IMAGE_PATH/*-{raspbian,raspios}-*-arm64-*.{zip,7z,xz} | head -n 1)
else
[ -n "$BASE_ZIP_IMG" ] || BASE_ZIP_IMG=`ls -t $BASE_IMAGE_PATH/*-{raspbian,raspios}*.{zip,7z,xz} | head -n 1`
[ -n "$BASE_ZIP_IMG" ] || BASE_ZIP_IMG=$(ls -t $BASE_IMAGE_PATH/*-{raspbian,raspios}*.{zip,7z,xz} | head -n 1)
fi
# Default user raspbian
[ -n "$BASE_USER" ] || BASE_USER=pi
Expand All @@ -49,7 +49,7 @@ fi
# [ -n "$BASE_CHROOT_SCRIPT_PATH" ] || BASE_CHROOT_SCRIPT_PATH=$BASE_SCRIPT_PATH/chroot_script
[ -n "$BASE_MOUNT_PATH" ] || BASE_MOUNT_PATH=$BASE_WORKSPACE/mount

if [ "${BASE_DISTRO}" == "ubuntu" ]; then
if [ "${BASE_DISTRO}" = "ubuntu" ]; then
[ -n "${BASE_BOOT_MOUNT_PATH}" ] || BASE_BOOT_MOUNT_PATH=boot/firmware
else
[ -n "${BASE_BOOT_MOUNT_PATH}" ] || BASE_BOOT_MOUNT_PATH=boot
Expand Down Expand Up @@ -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`
Copy link
Owner

@guysoft guysoft Oct 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@umlaeute
This seems to break now for me, not sure why, investigating

Copy link
Contributor Author

@umlaeute umlaeute Oct 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

🤔

Copy link
Contributor Author

@umlaeute umlaeute Oct 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my reference to docker was triggered by #206 (comment) (for whatever reasons I assumed that the comment on is related to this regression)

Copy link
Contributor Author

@umlaeute umlaeute Oct 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you provide an example (or link to a repository) that shows the problem?

Copy link
Owner

@guysoft guysoft Oct 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related:
#209

Copy link
Owner

@guysoft guysoft Nov 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Contributor Author

@umlaeute umlaeute Nov 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

[ -n "$BASE_COMMIT" ] || BASE_COMMIT=$(git -C "${CUSTOM_PI_OS_PATH}" rev-parse HEAD)

#Memory split
[ -n "$BASE_CONFIG_MEMSPLIT" ] || BASE_CONFIG_MEMSPLIT=default
Expand All @@ -107,3 +107,7 @@ fi

# Enable uart console on boot
[ -n "$BASE_ENABLE_UART" ] || BASE_ENABLE_UART=no


# Clean apt-cache after all the work is done
: ${BASE_APT_CLEAN:=yes}
4 changes: 3 additions & 1 deletion src/modules/base/end_chroot_script
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,5 +39,7 @@ if [ "${BASE_DISTRO}" == "ubuntu" ];then
fi

#cleanup
apt-get clean
if [ "${BASE_APT_CLEAN}" = "yes" ]; then
apt-get clean
fi
apt-get autoremove -y