<div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr">On Wed, May 15, 2019 at 7:03 PM Jeffery To <<a href="mailto:jeffery.to@gmail.com">jeffery.to@gmail.com</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr">On Fri, May 3, 2019 at 5:40 PM Petr Štetiar <<a href="mailto:ynezz@true.cz" target="_blank">ynezz@true.cz</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Jeffery To <<a href="mailto:jeffery.to@gmail.com" target="_blank">jeffery.to@gmail.com</a>> [2019-05-03 01:33:27]:<br>
<br>
> When looping through a package's STAGING_FILES_LIST (a list of<br>
> file/directory paths delimited by newlines), if the path contains<br>
> spaces, then the path will be split by the while loops, and the<br>
> file/directory will not be deleted/removed.<br>
> <br>
> This sets the internal field separator to the newline only so that the<br>
> entire path is considered when deleting/removing.<br>
> <br>
> Signed-off-by: Jeffery To <<a href="mailto:jeffery.to@gmail.com" target="_blank">jeffery.to@gmail.com</a>><br>
> ---<br>
> scripts/clean-package.sh | 2 ++<br>
> 1 file changed, 2 insertions(+)<br>
> <br>
> diff --git a/scripts/clean-package.sh b/scripts/clean-package.sh<br>
> index e580566a52..3a824929c6 100755<br>
> --- a/scripts/clean-package.sh<br>
> +++ b/scripts/clean-package.sh<br>
> @@ -1,4 +1,6 @@<br>
> #!/usr/bin/env bash<br>
> +IFS="<br>
> +"<br>
> [ -n "$1" -a -n "$2" ] || {<br>
> echo "Usage: $0 <file> <directory>"<br>
> exit 1<br>
<br>
just for the record, copy&pasting my comment from your PR[1]:<br>
<br>
I'm just wondering, if this proposed fix with this strange looking IFS is the<br>
right direction. Usually if I've problem with whitespaces in filenames, it's<br>
because I've forget to use xargs. If I'm not mistaken, this [ -n "$entry" ] ||<br>
break construct could be replaced with xargs -r, there's even XARGS:=xargs -r<br>
in <a href="http://rules.mk" rel="noreferrer" target="_blank">rules.mk</a>. Just my two cents.<br>
<br>
1. <a href="https://github.com/openwrt/openwrt/pull/1806#issuecomment-475454138" rel="noreferrer" target="_blank">https://github.com/openwrt/openwrt/pull/1806#issuecomment-475454138</a><br>
<br>
-- ynezz<br></blockquote><div><br></div><div>For the record, here is my reply from the PR[1]:</div><div><br></div><div>IFS[2] is a standard internal variable that controls how the shell splits tokens. Setting it here determines how the while loops further down the script will split the input (lines of file paths generated by find). Normally, IFS contains space, newline, and tab, which causes the while loops to incorrectly split paths with spaces. This problem is avoided by setting IFS to the newline only.<br><br>The line you cite ([ -n "$entry" ] || break) causes the while loops (which delete the input file paths) to exit on the first empty line of input. It would make no sense to replace it with xargs.</div><div><br></div><div>* * *</div><div><br></div><div>I finally remembered, I copied this syntax from package/network/services/openvpn/files/openvpn.init[3]:<br></div><div><br></div><div>LIST_SEP="<br>"<br><br></div><div>then further down it's used in append_param()[4] as IFS="$LIST_SEP" (I won't quote the whole for loop here).</div><div><br></div><div>If you prefer, I can change this to use ksh93 syntax[5]:</div><div><br></div><div>IFS=$'\n'<br></div><div><br></div><div>If you want to rewrite the script to use xargs, please be my guest, but setting IFS solves the issue here of spaces in file paths (technically, lines of text read from a file list).<br></div><div><br></div><div><br></div><div>1. <a href="https://github.com/openwrt/openwrt/pull/1806#issuecomment-475506440" target="_blank">https://github.com/openwrt/openwrt/pull/1806#issuecomment-475506440</a></div><div>2. <a href="https://www.tldp.org/LDP/abs/html/internalvariables.html#IFSREF" target="_blank">https://www.tldp.org/LDP/abs/html/internalvariables.html#IFSREF</a></div><div>3. <a href="https://github.com/openwrt/openwrt/blob/172b02c05f2dc199f511f1577aeddfabb7790dc0/package/network/services/openvpn/files/openvpn.init#L13-L14" target="_blank">https://github.com/openwrt/openwrt/blob/172b02c05f2dc199f511f1577aeddfabb7790dc0/package/network/services/openvpn/files/openvpn.init#L13-L14</a></div><div>4. <a href="https://github.com/openwrt/openwrt/blob/172b02c05f2dc199f511f1577aeddfabb7790dc0/package/network/services/openvpn/files/openvpn.init#L43" target="_blank">https://github.com/openwrt/openwrt/blob/172b02c05f2dc199f511f1577aeddfabb7790dc0/package/network/services/openvpn/files/openvpn.init#L43</a></div><div>5. <a href="https://unix.stackexchange.com/a/184867" target="_blank">https://unix.stackexchange.com/a/184867</a></div><div><br></div></div></div></div></div></div></div></div></div></div></div></div></div></blockquote><div><br></div><div>Just want to add, it may not be obvious why there would be spaces in file paths (it wasn't obvious to me at first).</div><div><br></div><div>Apparently, some Go packages that users have been trying to build (Docker[1], Delve[2]) include files with spaces in their names (mostly as test cases for handling files with spaces in their name, I think). The Go compilation issue was eventually fixed, but I noticed that the files were not cleaned up properly from STAGING_DIR.<br></div><div><br></div><div><br></div><div>1. <a href="https://github.com/openwrt/packages/pull/7127#issuecomment-439688448">https://github.com/openwrt/packages/pull/7127#issuecomment-439688448</a></div><div>2. <a href="https://github.com/openwrt/packages/issues/7639">https://github.com/openwrt/packages/issues/7639</a></div><div><br></div></div></div></div></div>