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

Framework: fix for wizard file permissions #5608

Merged
merged 2 commits into from
Feb 17, 2023

Conversation

mreid-tt
Copy link
Contributor

@mreid-tt mreid-tt commented Feb 17, 2023

Description

In some builds that included wizard UI files based on shell scripts, the shell scripts didn't execute during the installation wizard under DSM6 because they lacked the execute bit set (#5524 (comment) and #5455 (comment)). This issue occurred because the current spksrc.spk.mk set the execute bit as part of a block that evaluated the contents of the parent folder based on wildcards. However, for Makefiles, wildcards are evaluated during parsing and are not re-evaluated when the containing statement is executed (see: The Trouble with $(wildcard)).

To solve this issue, we define a new variable called WIZARD_FILE_LIST that generates a list of files in the directory. Then we reference this variable from the wizards target in an if statement to check if the variable has a value. This approach ensures that the variable has the correct value when evaluated, and we no longer rely on wildcards to evaluate the contents of the parent folder.

To solve this issue, the code was modified to check the existence of the DSM_WIZARDS_DIR directory instead. If the WIZARDS_DIR variable was already evaluated as not null, this directory would exist, ensuring that the files within it are consistently processed for file permissions. This approach removes the reliance on wildcards to evaluate the contents of the parent folder and ensures that the shell scripts have the correct file permissions during the installation wizard.

Follow up of #5383
Fixes #

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)

mk/spksrc.spk.mk Outdated Show resolved Hide resolved
mk/spksrc.spk.mk Outdated Show resolved Hide resolved
mk/spksrc.spk.mk Outdated Show resolved Hide resolved
Co-Authored-By: Antoine Malliarakis <antoine.malliarakis@gmail.com>
Copy link
Contributor

@th0ma7 th0ma7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mreid-tt mreid-tt merged commit e1ee048 into SynoCommunity:master Feb 17, 2023
@mreid-tt mreid-tt deleted the framework-patch branch February 17, 2023 20:35
@mreid-tt mreid-tt mentioned this pull request Mar 24, 2023
9 tasks
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.

3 participants