Skip to contentSkip to conversation

fs/common.mk: use find instead of shell glob patterns

PATCH Applied Mathieu Mirmont via buildroot 5 participants 13 messages Mar 10, 2022 – Mar 19, 2022 VFS

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 fs/common.mk: use find instead of shell glob patterns

Different shells can have different behaviours when it comes to globbing patterns. The dash shell (/bin/sh) on Debian testing switched to a different fnmatch/glob implementation that results in this new behaviour:

Using bash:

$ mkdir /tmp/foo
$ echo /tmp/foo/.[^.]*
/tmp/foo/.[^.]*

Using dash:

$ mkdir /tmp/foo
$ echo /tmp/foo/.[^.]*
/tmp/foo/..

The current FAKEROOT script uses this shell glob pattern which now fails on recent Debian testing systems:

rm: refusing to remove '.' or '..' directory: skipping '/build/buildroot-fs/cpio/target/run/..'
rm: refusing to remove '.' or '..' directory: skipping '/build/buildroot-fs/cpio/target/tmp/..'

It seems safer to use find | xargs rm here instead of relying on shell globbing patterns.

---
 fs/common.mk | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
fs/common.mk +2 -1
index 45beb5ae7b..37eafac4f7 100644
@@ -186,7 +186,8 @@ $$(BINARIES_DIR)/$$(ROOTFS_$(2)_FINAL_IMAGE_NAME): $$(ROOTFS_$(2)_DEPENDENCIES)
 
 	$$(foreach hook,$$(ROOTFS_$(2)_PRE_GEN_HOOKS),\
 		$$(call PRINTF,$$($$(hook))) >> $$(FAKEROOT_SCRIPT)$$(sep))
-	echo "rm -rf $$(TARGET_DIR)/run/* $$(TARGET_DIR)/run/.[^.]* $$(TARGET_DIR)/tmp/* $$(TARGET_DIR)/tmp/.[^.]*" >> $$(FAKEROOT_SCRIPT)
+	echo "find $$(TARGET_DIR)/run/ -mindepth 1 -prune -print0 | xargs -0r rm -rf --" >> $$(FAKEROOT_SCRIPT)
+	echo "find $$(TARGET_DIR)/tmp/ -mindepth 1 -prune -print0 | xargs -0r rm -rf --" >> $$(FAKEROOT_SCRIPT)
 	$$(call PRINTF,$$(ROOTFS_REPRODUCIBLE)) >> $$(FAKEROOT_SCRIPT)
 	$$(call PRINTF,$$(ROOTFS_SELINUX)) >> $$(FAKEROOT_SCRIPT)
 	$$(call PRINTF,$$(ROOTFS_$(2)_CMD)) >> $$(FAKEROOT_SCRIPT)
Signature · 1 line
2.34.1

_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot

#1 lore raw

From: Mathieu Mirmont

Quoted 8 lines

Sent: 10 March 2022 10:18

Different shells can have different behaviours when it comes to globbing patterns. The dash shell (/bin/sh) on Debian testing switched to a different fnmatch/glob implementation that results in this new behaviour:

Using bash:

Quoted 8 lines
$ mkdir /tmp/foo
$ echo /tmp/foo/.[^.]*
/tmp/foo/.[^.]*

Using dash:

$ mkdir /tmp/foo
$ echo /tmp/foo/.[^.]*
/tmp/foo/..

That is just broken.
Does the [^x] pattern work at all?

Raise a bug on 'Debian testing' hopefully they'll
fix it before it actually gets released anywhere.

For this script I'd just ignore errors from rm.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot

#2 lore raw
Quoted 18 lines · David Laight

From: Mathieu Mirmont

Quoted 8 lines

Sent: 10 March 2022 10:18

Different shells can have different behaviours when it comes to globbing patterns. The dash shell (/bin/sh) on Debian testing switched to a different fnmatch/glob implementation that results in this new behaviour:

Using bash:

Quoted 8 lines
$ mkdir /tmp/foo
$ echo /tmp/foo/.[^.]*
/tmp/foo/.[^.]*

Using dash:

$ mkdir /tmp/foo
$ echo /tmp/foo/.[^.]*
/tmp/foo/..
Quoted 5 lines

That is just broken.
Does the [^x] pattern work at all?

Raise a bug on 'Debian testing' hopefully they'll
fix it before it actually gets released anywhere.

The new behaviour of dash is very strange indeed, but also according to POSIX the wildcard pattern [^x] is undefined so we should not rely on it.

Signature · 1 line
Mat <[email protected]>

_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot

#3 lore raw

From: Mat

Quoted 24 lines

Sent: 10 March 2022 11:33

Quoted 21 lines · David Laight

From: Mathieu Mirmont

Quoted 8 lines

Sent: 10 March 2022 10:18

Different shells can have different behaviours when it comes to globbing patterns. The dash shell (/bin/sh) on Debian testing switched to a different fnmatch/glob implementation that results in this new behaviour:

Using bash:

Quoted 8 lines
$ mkdir /tmp/foo
$ echo /tmp/foo/.[^.]*
/tmp/foo/.[^.]*

Using dash:

$ mkdir /tmp/foo
$ echo /tmp/foo/.[^.]*
/tmp/foo/..

That is just broken.
Does the [^x] pattern work at all?

Quoted 6 lines
Quoted 2 lines

Raise a bug on 'Debian testing' hopefully they'll
fix it before it actually gets released anywhere.

The new behaviour of dash is very strange indeed, but also according to POSIX the wildcard pattern [^x] is undefined so we should not rely on it.

Well [^x] originally meant '^' or 'x' (the same as [x^] but then shells started treating [^x] the same as [!x].
So I guess POSIX has allowed such shells to be conformant by making [^x] undefined.

I'm not sure how many shells treat [^x] the same as [!x] but enough do that people (including me) have forgotten the difference. ash from a recent buildroot treats [^x] as [!x} but a cygwin ash I've got doesn't.
A quick search failed to find dash.

ISTR dash is badly broken in other places, try:

$ x=aaab234; echo ${x##${x%%b*}}
b234

It is worth checking whether Debian have changed the behaviour of dash or whether they've just changed the default root shell to dash.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot

#4 lore raw
Quoted 2 lines · David Laight

It is worth checking whether Debian have changed the behaviour of dash or whether they've just changed the default root shell to dash.

They've switched to the internal fnmatch implementation of dash [1] in this commit [1]. Before that they were using fnmatch(3) from the glibc.

[1] https://salsa.debian.org/debian/dash/-/commit/d7b1269eeeb5749ccc577f6e01dfccc2916b0130

Signature · 1 line
Mathieu Mirmont <[email protected]>

_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot

#5 lore raw

From: Mathieu Mirmont

Quoted 11 lines

Sent: 10 March 2022 12:11

Quoted 2 lines · David Laight

It is worth checking whether Debian have changed the behaviour of dash or whether they've just changed the default root shell to dash.

They've switched to the internal fnmatch implementation of dash [1] in this commit [1]. Before that they were using fnmatch(3) from the glibc.

[1] https://salsa.debian.org/debian/dash/-/commit/d7b1269eeeb5749ccc577f6e01dfccc2916b0130

Hmmm:

External implementations change in subtle ways, leading to hard to diagnose bugs. It’s better to rely on the built-in implementation instead.

As does randomly changing the implementation!

Anyone signed up to the debian lists to comment?

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot

#6 lore raw
Quoted 13 lines · David Laight

From: Mathieu Mirmont

Quoted 11 lines

Sent: 10 March 2022 12:11

Quoted 2 lines · David Laight

It is worth checking whether Debian have changed the behaviour of dash or whether they've just changed the default root shell to dash.

They've switched to the internal fnmatch implementation of dash [1] in this commit [1]. Before that they were using fnmatch(3) from the glibc.

[1] https://salsa.debian.org/debian/dash/-/commit/d7b1269eeeb5749ccc577f6e01dfccc2916b0130

Quoted 7 lines

Hmmm:

External implementations change in subtle ways, leading to hard to diagnose bugs. It’s better to rely on the built-in implementation instead.

As does randomly changing the implementation!

True, however [^x] is not part of POSIX so it can't be expected to work with /bin/sh.

Signature · 1 line
Mathieu Mirmont <[email protected]>

_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot

#7 lore raw
In reply to Mathieu Mirmont via buildroot
Quoted 4 lines · Mathieu Mirmont

Using dash:

$ mkdir /tmp/foo
$ echo /tmp/foo/.[^.]*
/tmp/foo/..

There is an error in this glob pattern. Fixing it would be simpler than invoking find/xargs.

Within a regexp, character classes are complemented with "^": [^.] means "any character other that a dot". Within a glob pattern, character classes are complemented with "!", and the same class would be written [!.]

C.f. glob(7).

Regards,

Edgar.
_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot

#8 lore raw

From: Of Edgar Bonet

Quoted 8 lines

Sent: 10 March 2022 11:22

Quoted 4 lines · Mathieu Mirmont

Using dash:

$ mkdir /tmp/foo
$ echo /tmp/foo/.[^.]*
/tmp/foo/..
Quoted 7 lines

There is an error in this glob pattern. Fixing it would be simpler than invoking find/xargs.

Within a regexp, character classes are complemented with "^": [^.] means "any character other that a dot". Within a glob pattern, character classes are complemented with "!", and the same class would be written [!.]

I should know that :-)

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)

_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot

#9 lore raw

PATCH v2 1/1 fs/common.mk: fix the globing pattern

In reply to Mathieu Mirmont via buildroot

The FAKEROOT script uses [^x] wildcard patterns which, while supported by many shells and interpreted like a regex, are undefined according to POSIX.

The dash shell (/bin/sh) on Debian testing switched to a different fnmatch/glob implementation that does not support [^x]. Instead it treats [^x] as either "^" or "x", and as a result buildroot fails to build on this distro:

rm: refusing to remove '.' or '..' directory: skipping '/build/buildroot-fs/cpio/target/run/..'
rm: refusing to remove '.' or '..' directory: skipping '/build/buildroot-fs/cpio/target/tmp/..'

The correct form should be [!x] rather than [^x].

---
 fs/common.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/common.mk +1 -1
index 45beb5ae7b..64a94d9ad8 100644
@@ -186,7 +186,7 @@ $$(BINARIES_DIR)/$$(ROOTFS_$(2)_FINAL_IMAGE_NAME): $$(ROOTFS_$(2)_DEPENDENCIES)
 
 	$$(foreach hook,$$(ROOTFS_$(2)_PRE_GEN_HOOKS),\
 		$$(call PRINTF,$$($$(hook))) >> $$(FAKEROOT_SCRIPT)$$(sep))
-	echo "rm -rf $$(TARGET_DIR)/run/* $$(TARGET_DIR)/run/.[^.]* $$(TARGET_DIR)/tmp/* $$(TARGET_DIR)/tmp/.[^.]*" >> $$(FAKEROOT_SCRIPT)
+	echo "rm -rf $$(TARGET_DIR)/run/* $$(TARGET_DIR)/run/.[!.]* $$(TARGET_DIR)/tmp/* $$(TARGET_DIR)/tmp/.[!.]*" >> $$(FAKEROOT_SCRIPT)
 	$$(call PRINTF,$$(ROOTFS_REPRODUCIBLE)) >> $$(FAKEROOT_SCRIPT)
 	$$(call PRINTF,$$(ROOTFS_SELINUX)) >> $$(FAKEROOT_SCRIPT)
 	$$(call PRINTF,$$(ROOTFS_$(2)_CMD)) >> $$(FAKEROOT_SCRIPT)
Signature · 3 lines
2.35.1
Changes v1 -> v2:
  - Fix the regex rather than use find|xargs (suggested by Edgar Bonet & David Laight)

_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot

#10 lore raw

Re: [Buildroot] [PATCH v2] fs/common.mk: fix the globing pattern

Mathieu, All,

On 2022-03-10 13:02 +0100, Mathieu Mirmont via buildroot spake thusly:

Quoted 24 lines

The FAKEROOT script uses [^x] wildcard patterns which, while supported by many shells and interpreted like a regex, are undefined according to POSIX.

The dash shell (/bin/sh) on Debian testing switched to a different fnmatch/glob implementation that does not support [^x]. Instead it treats [^x] as either "^" or "x", and as a result buildroot fails to build on this distro:

rm: refusing to remove '.' or '..' directory: skipping '/build/buildroot-fs/cpio/target/run/..'
rm: refusing to remove '.' or '..' directory: skipping '/build/buildroot-fs/cpio/target/tmp/..'

The correct form should be [!x] rather than [^x].

---
 fs/common.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/common.mk +0 -0
index 45beb5ae7b..64a94d9ad8 100644

fs/common.mk $$(BINARIES_DIR)/$$(ROOTFS_$(2)_FINAL_IMAGE_NAME): $$(ROOTFS_$(2)_DEPENDENCIES) · line 189

Quoted 5 lines
fs/common.mk +1 -1
@@ -186,7 +186,7 @@ $$(BINARIES_DIR)/$$(ROOTFS_$(2)_FINAL_IMAGE_NAME): $$(ROOTFS_$(2)_DEPENDENCIES)
 
 	$$(foreach hook,$$(ROOTFS_$(2)_PRE_GEN_HOOKS),\
 		$$(call PRINTF,$$($$(hook))) >> $$(FAKEROOT_SCRIPT)$$(sep))
-	echo "rm -rf $$(TARGET_DIR)/run/* $$(TARGET_DIR)/run/.[^.]* $$(TARGET_DIR)/tmp/* $$(TARGET_DIR)/tmp/.[^.]*" >> $$(FAKEROOT_SCRIPT)
+	echo "rm -rf $$(TARGET_DIR)/run/* $$(TARGET_DIR)/run/.[!.]* $$(TARGET_DIR)/tmp/* $$(TARGET_DIR)/tmp/.[!.]*" >> $$(FAKEROOT_SCRIPT)

I think I preferred your find-based solution, because, as far as I understand, this is still broken, as it won't match files named, for example "..foo"

The find-based solution, though, should work for everything and does not risk any issue with badly interpreted blobs in varios shells and various shell versions.

Regards,
Yann E. MORIN.

Quoted 15 lines
fs/common.mk +0 -0
 	$$(call PRINTF,$$(ROOTFS_REPRODUCIBLE)) >> $$(FAKEROOT_SCRIPT)
 	$$(call PRINTF,$$(ROOTFS_SELINUX)) >> $$(FAKEROOT_SCRIPT)
 	$$(call PRINTF,$$(ROOTFS_$(2)_CMD)) >> $$(FAKEROOT_SCRIPT)
Signature · 11 lines
2.35.1
Changes v1 -> v2:
  - Fix the regex rather than use find|xargs (suggested by Edgar Bonet & David Laight)




_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot
Signature · 10 lines
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot
#11 lore raw
In reply to Mathieu Mirmont via buildroot

Mathieu, All,

On 2022-03-10 11:18 +0100, Mathieu Mirmont via buildroot spake thusly:

Quoted 18 lines

Different shells can have different behaviours when it comes to globbing patterns. The dash shell (/bin/sh) on Debian testing switched to a different fnmatch/glob implementation that results in this new behaviour:

Using bash:

$ mkdir /tmp/foo
$ echo /tmp/foo/.[^.]*
/tmp/foo/.[^.]*

Using dash:

$ mkdir /tmp/foo
$ echo /tmp/foo/.[^.]*
/tmp/foo/..

The current FAKEROOT script uses this shell glob pattern which now fails on recent Debian testing systems:

Quoted 7 lines

rm: refusing to remove '.' or '..' directory: skipping '/build/buildroot-fs/cpio/target/run/..'
rm: refusing to remove '.' or '..' directory: skipping '/build/buildroot-fs/cpio/target/tmp/..'

It seems safer to use find | xargs rm here instead of relying on shell globbing patterns.

I applied to master, after adding a little blurb about the ..foo files that would be missed with globs.

Quoted 9 lines
---
 fs/common.mk | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
fs/common.mk +0 -0
index 45beb5ae7b..37eafac4f7 100644

fs/common.mk $$(BINARIES_DIR)/$$(ROOTFS_$(2)_FINAL_IMAGE_NAME): $$(ROOTFS_$(2)_DEPENDENCIES) · line 190

Quoted 6 lines
fs/common.mk +2 -1
@@ -186,7 +186,8 @@ $$(BINARIES_DIR)/$$(ROOTFS_$(2)_FINAL_IMAGE_NAME): $$(ROOTFS_$(2)_DEPENDENCIES)
 
 	$$(foreach hook,$$(ROOTFS_$(2)_PRE_GEN_HOOKS),\
 		$$(call PRINTF,$$($$(hook))) >> $$(FAKEROOT_SCRIPT)$$(sep))
-	echo "rm -rf $$(TARGET_DIR)/run/* $$(TARGET_DIR)/run/.[^.]* $$(TARGET_DIR)/tmp/* $$(TARGET_DIR)/tmp/.[^.]*" >> $$(FAKEROOT_SCRIPT)
+	echo "find $$(TARGET_DIR)/run/ -mindepth 1 -prune -print0 | xargs -0r rm -rf --" >> $$(FAKEROOT_SCRIPT)
+	echo "find $$(TARGET_DIR)/tmp/ -mindepth 1 -prune -print0 | xargs -0r rm -rf --" >> $$(FAKEROOT_SCRIPT)

That could have been a single call to find:

    find $$(TARGET_DIR)/run/ $$(TARGET_DIR)/tmp/ -mindepth 1 [...]

but there not much loss calling it twice, and the lines are shorter, which is nicer too, so I kept it as is.

Applied to master, thanks.

Regards,
Yann E. MORIN.

Quoted 13 lines
fs/common.mk +0 -0
 	$$(call PRINTF,$$(ROOTFS_REPRODUCIBLE)) >> $$(FAKEROOT_SCRIPT)
 	$$(call PRINTF,$$(ROOTFS_SELINUX)) >> $$(FAKEROOT_SCRIPT)
 	$$(call PRINTF,$$(ROOTFS_$(2)_CMD)) >> $$(FAKEROOT_SCRIPT)
Signature · 9 lines
2.34.1




_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot
Signature · 10 lines
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot
#12 lore raw
In reply to Mathieu Mirmont via buildroot
Quoted 20 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Mathieu" == Mathieu Mirmont via buildroot <[email protected]> writes:

Different shells can have different behaviours when it comes to globbing patterns. The dash shell (/bin/sh) on Debian testing switched to a different fnmatch/glob implementation that results in this new behaviour:

Using bash:

$ mkdir /tmp/foo
$ echo /tmp/foo/.[^.]*
/tmp/foo/.[^.]*

Using dash:

$ mkdir /tmp/foo
$ echo /tmp/foo/.[^.]*
/tmp/foo/..

The current FAKEROOT script uses this shell glob pattern which now fails on recent Debian testing systems:

Quoted 7 lines

rm: refusing to remove '.' or '..' directory: skipping '/build/buildroot-fs/cpio/target/run/..'
rm: refusing to remove '.' or '..' directory: skipping '/build/buildroot-fs/cpio/target/tmp/..'

It seems safer to use find | xargs rm here instead of relying on shell globbing patterns.

Committed to 2022.02.x, thanks.

Signature · 5 lines
Bye, Peter Korsgaard
_______________________________________________
buildroot mailing list
[email protected]
https://lists.buildroot.org/mailman/listinfo/buildroot