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

fix package user in privilege file #4781

Merged
merged 2 commits into from
Aug 18, 2021

Conversation

hgy59
Copy link
Contributor

@hgy59 hgy59 commented Aug 15, 2021

Motivation: jq does not create valid json when username is already set
Linked issues: This error popped up with the publish of #4602

Checklist

  • Build rule all-supported completed successfully
  • Package upgrade completed successfully
  • New installation of package completed successfully

With already set "username": "USER" jq appends an } at the end of the privilege file and thus makes it invalid.

@hgy59
Copy link
Contributor Author

hgy59 commented Aug 15, 2021

@publicarray this is a very strange behaviour of jq.
It might be related to the inplace modification with 1<>$@ , but I have no clue.

@hgy59
Copy link
Contributor Author

hgy59 commented Aug 15, 2021

Without the fix, the privilege file was created like this:
privilege.txt

@publicarray
Copy link
Member

Yes you are 100% right. Same problem/solution here as well: 7edf5f4

we might be better off adding the sponge utility from the moreutils package -> https://stackoverflow.com/questions/42716734/modify-a-key-value-in-a-json-using-jq-in-place

@publicarray
Copy link
Member

publicarray commented Aug 15, 2021

man sponge (I've not heard of sponge before)

sponge - soak up standard input and write to a file

sed '...' file | grep '...' | sponge file

sponge reads standard input and writes it out to the specified file. Unlike a shell redirect, sponge soaks up all its input before opening the output file. This allows constricting pipelines that read from and write to the same file.

If no output file is specified, sponge outputs to stdout.

@hgy59 hgy59 mentioned this pull request Aug 15, 2021
3 tasks
@hgy59
Copy link
Contributor Author

hgy59 commented Aug 15, 2021

@publicarray it runs like a charm with sponge...
The dockerfile update is in a dedicated PR #4782, as the updated spksrc image must be available when we merge #4781.

@hgy59
Copy link
Contributor Author

hgy59 commented Aug 15, 2021

Same problem/solution here as well: 7edf5f4

Thanks for the link, I remembered little, but couldn't find it...

@publicarray
Copy link
Member

Thanks @hgy59 and no problem 😄

mk/spksrc.service.mk Outdated Show resolved Hide resolved
@hgy59
Copy link
Contributor Author

hgy59 commented Aug 15, 2021

@publicarray now it would be great use jq for the generation of $(STAGING_DIR)/$(DSM_UI_DIR)/config: to avoid the python -m json.tool

@publicarray publicarray merged commit 5b6a258 into SynoCommunity:master Aug 18, 2021
@hgy59 hgy59 deleted the fix_privilege branch August 20, 2021 04:51
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.

2 participants