Bug 11191

Summary:	xattr and check-package issue
Product:	buildroot	Reporter:	Jean-pierre Cartal <jpcartal>
Component:	Other	Assignee:	Ricardo Martincoski <ricardo.martincoski>
Status:	RESOLVED FIXED
Severity:	normal	CC:	buildroot, yann.morin.1998
Priority:	P5
Version:	2018.02.2
Target Milestone:	---
Hardware:	All
OS:	Linux
Host:		Target:
Build:

Description Jean-pierre Cartal 2018-07-25 15:24:32 UTC

check-package warns about missing indent with xattr line in permissions of packages, however adding indentation to those lines makes the check-package script happy but then corresponding xattr entries are then silently ignored when creating the image.

e.g., this entry will make check-package issue a warning :

...
define TEST_PERMISSIONS
	/usr/bin/test f 0755 user0 users - - - - -
|xattr cap_kill+eip
|xattr cap_sys_nice+eip
|xattr cap_sys_time+eip

...

package/test/test.mk:618: expected indent with tabs

However changing it to

define TEST_PERMISSIONS
	/usr/bin/test f 0755 user0 users - - - - -
\t|xattr cap_kill+eip
\t|xattr cap_sys_nice+eip
\t|xattr cap_sys_time+eip

...

Will make check-package happy but xattr attributes will not be taken into account without any error message.

Comment 1 Ricardo Martincoski 2018-07-26 00:42:16 UTC

I can reproduce both behaviors on current master:
- check-package complaining when tab is not used for |xattr
- xattr being silently ignored by host-makedevs when tab is used for |xattr

The second one occurs because the tab propagates from the package recipe to package/pkg-generic.mk to fs/common.mk and finally to output/build/buildroot-fs/device_table.txt and the code that processes this file does a strict check:
line 513 @ package/makedevs/makedevs.c
if (1 == sscanf(line, "|xattr %254s", xattr)) {

We could fix this inconsistency by a number of ways:
1) change check-package;
2) try to come up with a solution in package/pkg-generic.mk to remove leading tabs from _PERMISSION. It cannot be a simple $$(strip) because it would remove the newlines;
3) change makedevs to ignore leading tabs/spaces for xattr (as it already does for non-xattr lines). Hackish version to replace the code mentioned above:
> if (2 == sscanf(line, "%4095s %254s", name, xattr) && !strcmp(name, "|xattr")) {
4) change fs/common.mk to remove the leading tabs/spaces. Something like this:
>+++ b/fs/common.mk
>@@@ -89,2 -89,2 +89,3 @@@ endi
>        $(call PRINTF,$(PACKAGES_PERMISSIONS_TABLE)) >> $(FULL_DEVICE_TABLE)
>++      $(SED) "s/^[ \t]*//g" $(FULL_DEVICE_TABLE)
>        echo "$(HOST_DIR)/bin/makedevs -d $(FULL_DEVICE_TABLE) $(TARGET_DIR)" >> $(FAKEROOT_SCRIPT)

I think option 4 makes sense because we also will end up with a better formatted device_table.txt.

But let's wait for more opinions on how to proceed.

Comment 2 Yann E. MORIN 2018-08-14 21:20:23 UTC

(In reply to Ricardo Martincoski from comment #1)

In addition to Ricardo's suggestions, here's I believe the
technically simplest solution:

5) Require the _PERMISSIONS to not be indented at all.

But I believe hat Ricardo's #3 is the best solution.

Comment 3 Ricardo Martincoski 2018-08-16 03:01:52 UTC

Fixed in the 2018.02.x branch:
https://git.buildroot.org/buildroot/commit/?h=2018.02.x&id=1369d30a99234b256c9b5bed08d7764393ccd09f
This commit will be part of the next 2018.02.5 release.

The same fix was applied to 2018.05.x and master branches, but on those branches there is another xattr issue that I found while fixing this one:
https://bugs.busybox.net/show_bug.cgi?id=11216