Add support for files with spaces in their names #24

Crown0815 · 2024-10-07T11:24:00Z

Crown0815 commented

2024-10-07 11:24:00 +00:00

So far when uploading files containing spaces, such as file 3.txt, the tea command will be broken due to incorrectly formatted tea arguments.
I have been receiving Remote repository required: Specify ID via --repo or execute from a local git repo. which was not helpful until I turned on verbose and saw the whole command.

This fix should resolve this issue and thereby add active support for files with spaces.

So far when uploading files containing spaces, such as `file 3.txt`, the `tea` command will be broken due to incorrectly formatted `tea` arguments. I have been receiving `Remote repository required: Specify ID via --repo or execute from a local git repo.` which was not helpful until I turned on `verbose` and saw the whole command. This fix should resolve this issue and thereby add active support for files with spaces.

earl-warren commented

2024-12-03 08:54:52 +00:00

It would be nice to fix this bug. Could you please resolve the conflict?

Crown0815 force-pushed main from b10940c29f

/ integration (pull_request) Failing after 1m46s

Details

to 7a1ac7c579

/ integration (pull_request) Failing after 2m10s

Details

2024-12-04 12:28:34 +00:00

Compare

earl-warren commented

2024-12-04 13:54:45 +00:00

The relevant error is at https://code.forgejo.org/actions/forgejo-release/actions/runs/430/jobs/0#jobstep-4-1284

Look for Failure - Main to find it. The logs are enormous.

The relevant error is at https://code.forgejo.org/actions/forgejo-release/actions/runs/430/jobs/0#jobstep-4-1284 Look for `Failure - Main` to find it. The logs are enormous.

Crown0815 added 2 commits

2024-12-04 15:35:14 +00:00

Resolve all SC2086 warnings 765e655aba

This should also resolve all issues related to spaces in file names

See https://github.com/koalaman/shellcheck/wiki/SC2086

Replace all tabs with spaces

/ integration (pull_request) Failing after 2m11s

Details

f4520558e6

According to .editorconfig file

Crown0815 commented

2024-12-04 15:36:47 +00:00

@earl-warren I think I found the issue. I went ahead and resolved even more potential issues due to spaces in file or directory names by resolving all SC2086 warnings.

@earl-warren I think I found the issue. I went ahead and resolved even more potential issues due to spaces in file or directory names by resolving all [SC2086](https://github.com/koalaman/shellcheck/wiki/SC2086) warnings.

👍 1 🎉 1

Crown0815 added 1 commit

2024-12-04 15:47:34 +00:00

Reactivate word splitting for passphrase

/ integration (pull_request) Failing after 2m9s

Details

c089625f57

The passphrase variable relies on word splitting to provide parameter key and value for gpg

Crown0815 commented

2024-12-04 16:03:00 +00:00

@earl-warren it appears that the tea cli has a problem with the -a "upload-dir/file 3.txt" option:

Incorrect Usage: flag provided but not defined: -a "upload-dir/file 3.txt"

I am not sure what the issue is. In the log there is this line which seems to indicate a ' being inserted into the command. But I do not see what in the code would cause a ' to appear.

The error is then reported here:

Incorrect Usage: flag provided but not defined: -a "upload-dir/file 3.txt"

@earl-warren it appears that the `tea` cli has a problem with the `-a "upload-dir/file 3.txt"` option: ``` Incorrect Usage: flag provided but not defined: -a "upload-dir/file 3.txt" ``` I am not sure what the issue is. In the log there is [this line](https://code.forgejo.org/actions/forgejo-release/actions/runs/434#jobstep-4-701) which seems to indicate a `'` being inserted into the command. But I do not see what in the code would cause a `'` to appear. The error is then reported [here](https://code.forgejo.org/actions/forgejo-release/actions/runs/434#jobstep-4-709): ``` Incorrect Usage: flag provided but not defined: -a "upload-dir/file 3.txt" ```

earl-warren requested changes

2024-12-04 16:07:18 +00:00

Dismissed

forgejo-release.sh Outdated

					
				@ -55,2 +54,2 @@

				    if ! $BIN_DIR/tea release create $assets --repo $REPO --note "$RELEASENOTES" --tag $TAG --title "$TITLE" --draft ${releasetype} >& $TMP_DIR/tea.log ; then

				        if grep --quiet 'Unknown API Error: 500' $TMP_DIR/tea.log && grep --quiet services/release/release.go:194 $TMP_DIR/tea.log ; then

				    anchor=$(echo "$TAG" | sed -e 's/^v//' -e 's/[^a-zA-Z0-9]/-/g')

				    if ! "$BIN_DIR"/tea release create "$assets" --repo $REPO --note "$RELEASENOTES" --tag $TAG --title "$TITLE" --draft ${releasetype} >& "$TMP_DIR"/tea.log ; then

I think you have an evaluation problem here. "$assset" will be a single argument.

👍 1

You want "$assets" to not be a single argument but multiple arguments. I suggest creating a temporary script otherwise properly quoting and evaluating will be either difficult or obscure.

cat > uploader <<EOF
"$BIN_DIR"/tea release create $assets -repo $REPO --note "$RELEASENOTES" --tag $TAG --title "$TITLE" --draft ${releasetype}
EOF

and then bash uploader. This way the quoted file names $assets will be properly evaluated.

You want "$assets" to not be a single argument but multiple arguments. I suggest creating a temporary script otherwise properly quoting and evaluating will be either difficult or obscure. ```sh cat > uploader <<EOF "$BIN_DIR"/tea release create $assets -repo $REPO --note "$RELEASENOTES" --tag $TAG --title "$TITLE" --draft ${releasetype} EOF ``` and then `bash uploader`. This way the quoted file names $assets will be properly evaluated.

I resolved the issue by creating an array for the assets. It is the recommended solution for SC2086, under the given circumstances

Crown0815 marked this conversation as resolved

Crown0815 force-pushed main from 1f5768043f

/ integration (pull_request) Failing after 2m9s

Details

to 615f72e7fa

/ integration (pull_request) Failing after 1m34s

Details

2024-12-04 16:31:21 +00:00

Compare

Crown0815 commented

2024-12-04 16:42:18 +00:00

@earl-warren the issue I have now is that the download still assumes a file name without space here:

jq --raw-output '.assets[] | "\(.name) \(.browser_download_url)"' < "$TMP_DIR"/assets.json | while read name url ; do
    curl --fail -H "Authorization: token $TOKEN" -o "$name" -L "$url"
done

The issue is likely the "\(.name) \(.browser_download_url)" pattern, but I am not sure what this means. jq is json query?

@earl-warren the issue I have now is that the download still assumes a file name without space here: ``` jq --raw-output '.assets[] | "\(.name) \(.browser_download_url)"' < "$TMP_DIR"/assets.json | while read name url ; do curl --fail -H "Authorization: token $TOKEN" -o "$name" -L "$url" done ``` The issue is likely the `"\(.name) \(.browser_download_url)"` pattern, but I am not sure what this means. `jq` is json query?

earl-warren commented

2024-12-04 17:04:15 +00:00

Try this instead:

 jq --raw-output '.assets[] | "\(.browser_download_url) \(.name)"' < "$TMP_DIR"/assets.json | while read url name  ; do
   curl --fail -H "Authorization: token $TOKEN" -o "$name" -L "$url"
 done

name being the last argument will get everything after the URLs, space and all.

The issue is likely the "\(.name) \(.browser_download_url)" pattern, but I am not sure what this means. jq is json query?

https://en.wikipedia.org/wiki/Jq_(programming_language)

Try this instead: ```sh jq --raw-output '.assets[] | "\(.browser_download_url) \(.name)"' < "$TMP_DIR"/assets.json | while read url name ; do curl --fail -H "Authorization: token $TOKEN" -o "$name" -L "$url" done ``` name being the last argument will get everything after the URLs, space and all. > The issue is likely the `"\(.name) \(.browser_download_url)"` pattern, but I am not sure what this means. `jq` is json query? https://en.wikipedia.org/wiki/Jq_(programming_language)

🚀 1

Crown0815 added 1 commit

2024-12-04 18:01:56 +00:00

Fix download not handling files with spaces in names correctly

/ integration (pull_request) Successful in 6m43s

Details

44fea6428c

Crown0815 commented

2024-12-04 18:09:01 +00:00

Try this instead:
 jq --raw-output '.assets[] | "\(.browser_download_url) \(.name)"' < "$TMP_DIR"/assets.json | while read url name  ; do
   curl --fail -H "Authorization: token $TOKEN" -o "$name" -L "$url"
 done
name being the last argument will get everything after the URLs, space and all.

The issue is likely the "\(.name) \(.browser_download_url)" pattern, but I am not sure what this means. jq is json query?

https://en.wikipedia.org/wiki/Jq_(programming_language)

That is a pretty good idea. Did the trick

> > > Try this instead: > > ```sh > jq --raw-output '.assets[] | "\(.browser_download_url) \(.name)"' < "$TMP_DIR"/assets.json | while read url name ; do > curl --fail -H "Authorization: token $TOKEN" -o "$name" -L "$url" > done > ``` > name being the last argument will get everything after the URLs, space and all. > > > The issue is likely the `"\(.name) \(.browser_download_url)"` pattern, but I am not sure what this means. `jq` is json query? > > > https://en.wikipedia.org/wiki/Jq_(programming_language) That is a pretty good idea. Did the trick

earl-warren requested changes

2024-12-05 06:30:57 +00:00

Dismissed

forgejo-release.sh Outdated

					
				@ -44,3 +44,3 @@

				upload_release() {

				    local assets=$(ls $RELEASE_DIR/* | sed -e 's/^/-a /')

				    local assets=()

It is an unusual construct but clever. Someone reading the script will no doubt wonder why it is not just a string. Could you add a comment above to explain why it is justified?

What about adding something like this as well?

assets is a list of arguments, some of which may contain space and need to be quoted like this -a "foo bar" -a "frob". It will be expanded later with "${assets[@]}" which will correctly preserve the separation of arguments and not include the quotes into them.

I'm sure I will have forgotten this trick in a month 😁

What about adding something like this as well? > assets is a list of arguments, some of which may contain space and need to be quoted like this `-a "foo bar" -a "frob"`. It will be expanded later with `"${assets[@]}"` which will correctly preserve the separation of arguments and not include the quotes into them. I'm sure I will have forgotten this trick in a month 😁

Do you prefer a more detailed description over the reference to the original source?
Just want to make sure you saw the change I already put in.

Do you prefer a more detailed description over the reference to the original source? Just want to make sure you saw the change I already put in.

Yes, I would favor a detailed description that clarify how it is relevant in this context in addition to the link to the source. Otherwise my future self will have a more difficult time remembering the rationale 🙏

Crown0815 marked this conversation as resolved

forgejo-release.sh Outdated

					
				@ -55,2 +57,2 @@

				    if ! $BIN_DIR/tea release create $assets --repo $REPO --note "$RELEASENOTES" --tag $TAG --title "$TITLE" --draft ${releasetype} >& $TMP_DIR/tea.log ; then

				        if grep --quiet 'Unknown API Error: 500' $TMP_DIR/tea.log && grep --quiet services/release/release.go:194 $TMP_DIR/tea.log ; then

				    anchor=$(echo "$TAG" | sed -e 's/^v//' -e 's/[^a-zA-Z0-9]/-/g')

				    if ! "$BIN_DIR"/tea release create "${assets[@]}" --repo $REPO --note "$RELEASENOTES" --tag $TAG --title "$TITLE" --draft ${releasetype} >& "$TMP_DIR"/tea.log ; then

Clever bashism 👍

❤️ 1

It is actually the proposed solution for SC2086, so all credit goes to the author of the SC2086 wiki page ;-)

It is actually the proposed solution for SC2086, so all credit goes to the author of the [SC2086 wiki page](https://github.com/koalaman/shellcheck/wiki/SC2086#exceptions) ;-)

Crown0815 marked this conversation as resolved

Crown0815 added 1 commit

2024-12-05 07:17:39 +00:00

Clarify purpose of array for accumulation of assets

/ integration (pull_request) Successful in 6m44s

Details

b035423bc4

Crown0815 added 3 commits

2024-12-05 07:24:14 +00:00

refactor: Fix typo in variable name cfe5d616d2

refactor: Resolve previously missed violation of SC2086 d67ba51741

refactor: Remove unused variable

/ integration (pull_request) Successful in 6m41s

Details

50f803ce6a

earl-warren added the

enhancement

label

2024-12-05 07:26:05 +00:00

earl-warren reviewed

2024-12-05 14:45:20 +00:00

forgejo-release.sh Outdated

					
				@ -173,3 +175,1 @@

					jq --raw-output '.assets[] | "\(.name) \(.browser_download_url)"' < $TMP_DIR/assets.json | while read name url ; do

					    curl --fail -H "Authorization: token $TOKEN" -o $name -L $url

					done

				     jq --raw-output '.assets[] | "\(.browser_download_url) \(.name)"' < "$TMP_DIR"/assets.json | while read url name  ; do