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 compatibility with POSIX shell in SRM (ash) #5570

Merged
merged 3 commits into from
Jan 31, 2023

Conversation

publicarray
Copy link
Member

@publicarray publicarray commented Jan 21, 2023

Description

Rather than using arrays I'm using the newline character for separation, it is more hacky but should still work. I still need to test if deluge works with this. I have confirmed that Cloudflared works with this change on the SRM.

Fixes #5548

Checklist

  • Build rule all-supported completed successfully
  • New installation of package completed successfully
  • Package upgrade completed successfully (Manually install the package again)
  • Package functionality was tested
  • Any needed documentation is updated/created

Type of change

  • Bug fix
  • New Package
  • Package update
  • Includes small framework changes
  • This change requires a documentation update (e.g. Wiki)

@publicarray publicarray requested a review from th0ma7 January 21, 2023 09:18
@publicarray publicarray changed the title Fix compatibility with POSIX shells from SRM busybox Fix compatibility with POSIX shell in SRM (ash) Jan 21, 2023
@publicarray
Copy link
Member Author

Alternatively if this approach is too hackey we could use a token character to separate multiple SERVICE_COMMANDs e.g. :

@hgy59
Copy link
Contributor

hgy59 commented Jan 21, 2023

@publicarray what about the reload_inst_variables function in installer.functions? It uses declare to inject the variables from the ${INST_VARIABLES} file and might fail on SRM too. (there is a fix in #5550 to declare globally (-g))

@publicarray
Copy link
Member Author

Thanks @hgy59 I'll take a look

@publicarray
Copy link
Member Author

I think using export should work instead of declare -g

@hgy59
Copy link
Contributor

hgy59 commented Jan 22, 2023

I think using export should work instead of declare -g

@publicarray you are right. The reload_inst_variables function was introduced with the minio package. There is no need to use declare and I have redesigned this function to take the file to read variable declarations as parameter.

As I already did a redesign to allow special characters in such variables (#5430) there is a conflict that must be resolved.

Finally PR #5555 can benefit of the new function load_variables_from_file.

publicarray and others added 3 commits January 22, 2023 14:33
- rename reload_inst_variables to load_variables_from_file with parameter
- make spksrc.service.installer.functions compatible with sh/ash
@hgy59 hgy59 added framework srm related to Synology Routers dsm 5 labels Jan 22, 2023
@hgy59 hgy59 mentioned this pull request Jan 22, 2023
8 tasks
@hgy59 hgy59 merged commit 07bb379 into SynoCommunity:master Jan 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dsm 5 framework srm related to Synology Routers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SRM version of Cloudflared not working
2 participants