Skip to content
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

allow packages to install *.o files into the image #4191

Merged
merged 3 commits into from
Feb 13, 2020

Conversation

HiassofT
Copy link
Member

@HiassofT HiassofT commented Feb 9, 2020

*.o files were silently removed in scripts/build (for no obvious reason) which prevents shipping *.o files in the LE image. Drop that so we can include IR BPF decoders in the image.

This drop revealed a bug in safe_remove, it's called with wildcards in several places but it only removed the first file so several *.o files from glibc ended up in /usr/lib.

Python3 installs a python.o file eg in /usr/lib/python3.7/config-3.7-x86_64-linux-gnu/python.o, delete this is post_makeinstall_target.

With these changes both RPi4 and Generic images built fine and didn't include any unwanted *.o files.

commit c468820 "scripts/build: cleanup"
started to silently remove *.o files from the installation without
giving an explanation why this should be needed.

Drop that as it prevents packages from including *.o files in the
image, which eg is needed to include IR BPF decoders in LibreELEC.

Packages which install *.o files that should not end up in the image
should manually remove these in post_makeinstall_target.

Signed-off-by: Matthias Reichl <hias@horus.com>
Several packages call safe_remove with a wildcard to remove
multiple files but safe_remove only deleted the first one.

Fix this by iterating over all arguments passed into safe_remove
so unwanted files don't end up in the image.

Signed-off-by: Matthias Reichl <hias@horus.com>
Signed-off-by: Matthias Reichl <hias@horus.com>
@MilhouseVH MilhouseVH merged commit 70b69eb into LibreELEC:master Feb 13, 2020
@lrusak
Copy link
Member

lrusak commented Feb 13, 2020

did you verify that no other object files were installed into the image with this change?

@MilhouseVH
Copy link
Contributor

I compared before/after images and didn't see any difference.

@lrusak
Copy link
Member

lrusak commented Feb 13, 2020

I compared before/after images and didn't see any difference.

👍

The other problem is that safe_remove was never supposed to be used with wildcards. The entire purpose of using safe_remove was to detect changes in the packaging when bumping package versions. How can we detect a difference when we just use a wildcard to detect something we don't even know exists?

I'd suggest changing the safe_remove behavior back and fixing all the wildcards instead.

@lrusak
Copy link
Member

lrusak commented Feb 15, 2020

bueller?

@MilhouseVH
Copy link
Contributor

I believe @HiassofT is busy this weekend... I'm not sure of the specifics that led to the safe_remove changes.

@HiassofT
Copy link
Member Author

The introduction of safe_remove converted several rm -f / rm -rf statements with wildcards to use safe_remove - see commit 2ca0df2. Then commit 56cbf64 did the same for glibc and a2ca748 added a safe_remove with wildcard for weston.

This leads to two problems: if safe_remove doesn't follow rm semantics (allowing multiple files to be removed in a single call) that's quite unexpected behavior. Even more so as the first examples of safe_remove being used did so with wildcards (which would have lead to files not being removed, compared to the previous rm, if additional files matching the patterns were being installed by updated package versions).

So I'd say safe_remove implementation should be as I fixed it with the PR. It's rather easy that someone adds safe_remove with a wildcard (potentially matching multiple files) in the future, and only deleting the first file is a bug. It can also be helpful to find out about no longer existing files, like we already got a bunch of:

safe_remove: path does not exist: [glibc]: /home/hias/private/libreelec/libreelec-master/build.LibreELEC-RPi4.arm-9.80-devel/install_pkg/glibc-2.30/usr/lib/*.map
safe_remove: path does not exist: [systemd]: /home/hias/private/libreelec/libreelec-master/build.LibreELEC-RPi4.arm-9.80-devel/install_pkg/systemd-244/usr/lib/systemd/system/*.target.wants/systemd-udev-hwdb-update.service
safe_remove: path does not exist: [systemd]: /home/hias/private/libreelec/libreelec-master/build.LibreELEC-RPi4.arm-9.80-devel/install_pkg/systemd-244/usr/lib/systemd/system/*.target.wants/systemd-user-sessions.service

If someone prefers to change safe_remove with wildcards back to rm please do that in a separate PR

@HiassofT HiassofT deleted the le10-fix-obj-remove branch January 16, 2021 23:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants