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

Copy over patches when checking out x_x #1968

Closed
wants to merge 1 commit into from

Conversation

orhtej2
Copy link
Contributor

@orhtej2 orhtej2 commented Oct 12, 2024

The problem

Apps using helpers v2.1 fail to apply patches, even properly moved to sources/patches/$source_id/

Solution

Copy over patches during checkout, look in the right directory.

PR Status

Tested once (worked!)

How to test

Install Wallabag from https://github.com/YunoHost-Apps/wallabag2_ynh/tree/move_patches, log in (should work with LDAP patch applied).

@zamentur
Copy link
Member

Maybe the idea was to move sources/patches/ to patches/ ?

@orhtej2
Copy link
Contributor Author

orhtej2 commented Oct 13, 2024

Maybe the idea was to move sources/patches/ to patches/ ?

May as well be, but copying over sources was an easier thing to do ;)

@JimboJoe
Copy link
Member

Hello, if I understand well, this fix for core would solve the never-ending forum thread about failing wallabag authentication (patches not applied at install/upgrade). I'm also surprised it didn't cause problems in other apps... 🤔

@orhtej2
Copy link
Contributor Author

orhtej2 commented Dec 31, 2024

This band-aid fix requires further investigation + some decision on app assets handling (the contents of extra_files directory).

Stay tuned I guess 🤷

@matjack1
Copy link

matjack1 commented Jan 1, 2025

I think this PR is not necessary as the fix should be done in Wallabagger: YunoHost-Apps/wallabag2_ynh#229

As stated in the docs here: https://doc.yunohost.org/en/packaging_apps_helpers_v2.1#ynh-setup-source the code is correct regarding the patches location:

Patches named patches/${src_id}/*.patch will be applied to $dest_dir

@orhtej2 orhtej2 closed this Jan 1, 2025
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.

5 participants