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

[autopatch] Automatic patch attempt for helpers 2.1 #193

Open
wants to merge 3 commits into
base: testing
Choose a base branch
from

Conversation

yunohost-bot
Copy link
Contributor

This is an automatic PR

This is an automated patch for the switch to helpers 2.1

If you are not familar with helpers 2.1, please check the discussion at https://forum.yunohost.org/t/new-experimental-2-1-helpers/30114

Please bear in mind that this is just an automated patch and it is very likely to not work out of the box.

In particular it is known to possibly create syntax issues or permission issues which may need to be resolved manually. You are absolutely encouraged to come discuss about this on the app packaging room if you are unsure how to fix a specific issue.

@yunohost-bot yunohost-bot changed the base branch from master to testing August 31, 2024 11:20
@yunohost-bot
Copy link
Contributor Author

!testme

@ericgaspar
Copy link
Member

!testme

@yunohost-bot
Copy link
Contributor Author

Living in the future, are we?
Test Badge

@yunohost-bot
Copy link
Contributor Author

Alrighty!
Test Badge

domain=$new_domain
path=$new_path
ynh_add_config --template="../conf/config.ini" --destination="$install_dir/config.ini"
ynh_config_add --template="config.ini" --destination="$install_dir/config.ini"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks weird, that needs testing


chmod 400 "$install_dir/config.ini"
chown $app:$app "$install_dir/config.ini"
ynh_config_add --template="config.ini" --destination="$install_dir/config.ini"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same warning about config.ini path

Copy link
Member

Choose a reason for hiding this comment

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

#REMOVEME? Assuming the file is setup using ynh_config_add, the proper chmod/chowns are now already applied and it shouldn't be necessary to tweak perms | chmod 400 "$install_dir/config.ini"

chmod 400
chown $app:

should be applied by the helper 2.1. If the app is writing to the config file then chmod 600 should be added to the script...

https://github.com/YunoHost/yunohost/blob/5d15c00d921927825a0bf98b0c5d872dac57d1b7/helpers/helpers.v2.1.d/templating#L44

_ynh_apply_default_permissions() {

Copy link
Collaborator

Choose a reason for hiding this comment

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

My warning was regarding the config.ini filepath (moving from ../config.ini to config.ini).

Copy link
Member

Choose a reason for hiding this comment

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

you mean from --template="../conf/config.ini" to --template="config.ini"?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes

Copy link
Member

Choose a reason for hiding this comment

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

../conf/ path is implied, therefore so no need to put the full path

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants