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

ltoize hijacks user patch mechanism #105

Closed
mleise opened this issue Apr 7, 2018 · 13 comments
Closed

ltoize hijacks user patch mechanism #105

mleise opened this issue Apr 7, 2018 · 13 comments

Comments

@mleise
Copy link

mleise commented Apr 7, 2018

Due to the symlinks put in place by ltoize no further user patches can be created for any of the 626 packages in dev-libs, dev-qt and www-client. Ideally the patches files themselves would be symlinked instead of parent directories.

@InBetweenNames
Copy link
Owner

I believe this is a misunderstanding. Emerge will create directories and symlink as needed, but if existing patches are already present in those directories or there are other subdirectories, only the relevant directories and files will be symlinked. This behaviour is consistent on removal as well. I have a number of user patches which are not relevant to LTO that coexist fine with ltoize, for example.

@InBetweenNames
Copy link
Owner

To be clear, the only restriction is that your user patch cannot be named with the same filename as an LTO patch in this repository. By convention, we use lto.patch as the filename.

@mleise
Copy link
Author

mleise commented Apr 7, 2018

etc-update showed me /etc/portage/patches/dev-libs as being replaced by a symlink. I already had a patch for dev-libs/weston in there. As I understand you, portage should have accepted the existing dev-libs directory and created symlinks for the three wayland versions in there. I'm using portage-2.3.24.

@InBetweenNames
Copy link
Owner

Yes, that's what it does for me at least. It only symlinks the things that don't already exist. I'm on the latest portage.

@mleise
Copy link
Author

mleise commented Apr 7, 2018

Tried again with portage-2.3.28 and got the same result. After the merge:

* IMPORTANT: config file '/etc/portage/patches/dev-libs' needs updating.

And in etc-update:

The following is the list of files which need updating, each
configuration file is followed by a list of possible replacement files.
1) /etc/portage/patches/dev-libs (1)
Please select a file to edit by entering the corresponding number.
              (don't use -3, -5, -7 or -9 if you're unsure what to do)
              (-1 to exit) (-3 to auto merge all files)
                           (-5 to auto-merge AND not use 'mv -i')
                           (-7 to discard all updates)
                           (-9 to discard all updates AND not use 'rm -i'): 1

Showing differences between /etc/portage/patches/dev-libs and /etc/portage/patches/._cfg0000_dev-libs
--- /var/tmp/etc-update-5120/symdiff-PEc/0	2018-04-08 01:04:21.688010243 +0200
+++ /var/tmp/etc-update-5120/symdiff-PEc/1	2018-04-08 01:04:21.688010243 +0200
@@ -1 +1 @@
-DIR: /etc/portage/patches/dev-libs
+SYM: /etc/portage/patches/dev-libs -> /usr/repos/lto-overlay/sys-config/ltoize/files/patches/dev-libs

-------------------------------------------------------------
NOTE: File is a symlink to another file. REPLACE recommended.
      The original file may simply have moved. Please review.
-------------------------------------------------------------

The content of dev-libs is an empty patch file inside "weston": /etc/portage/patches/dev-libs/weston/lol.patch
In any case it looks like my portage is trying to replace the parent directory dev-libs.
My FEATURES is set to: assume-digests binpkg-logs ccache clean-logs config-protect-if-modified distlocks ebuild-locks fixlafiles merge-sync multilib-strict news parallel-fetch preserve-libs protect-owned sandbox sfperms strict unknown-features-warn unmerge-logs unmerge-orphans userfetch userpriv usersandbox usersync

@InBetweenNames
Copy link
Owner

What happens if you allow it to proceed?

@mleise
Copy link
Author

mleise commented Apr 8, 2018

You mean etc-update? It fails to do anything and just repeats the prompt, i.e. asks me again to replace, delete the update or merge (just like with any other file). But that was with stable portage. (I just realized that etc-update is part of the portage package.) I'll try again with ~amd64 portage.

@mleise
Copy link
Author

mleise commented Apr 8, 2018

Just checked that portage-2.3.28 etc-update works the same and yes it does. The only option that makes etc-update not get stuck in a menu loop is 2) Delete update, keeping original as is.

@InBetweenNames
Copy link
Owner

Good to know. The nice thing about user patches is that we don't need to fork ebuilds in this repo and keep things in sync constantly. Unfortunately, the user patching mechanism isn't really extensible, so the symlink solution was chosen as a compromise. It would be nice to have a portage plugin to do this, but my time is very limited at the moment. Perhaps it would be best, for now, to mandate that out of tree LTO patches be either a) upstreamed or b) forked here until the issue is resolved upstream.

@mleise
Copy link
Author

mleise commented Apr 14, 2018

I believe we know what the right thing™ is. But I don't feel the pressure to act now as I was able to remove the one conflicting patch directory and things just work™ now.

@InBetweenNames
Copy link
Owner

An alternative: replicate the user patches functionality with bashrc.d, and source LTO patches directly from the overlay, applied before /etc/patches. This should completely resolve the issue. Additional LTO patches can be tested first in /etc/patches before submitting them as a PR here.

InBetweenNames added a commit that referenced this issue Oct 27, 2018
LTO patches are now applied directly from lto-overlay,
using functionality replicated directly from portage.
/etc/patches is now left untouched by LTOize, and
a script, 41-lto-patch.sh, is symlinked into your
portage bashrc.d.

You can experiment with your own LTO patches in /etc/patches
as usual.  Please consider submitting them upstream if you
make working ones.

Also, with this change, we no longer need to increment the version
number with LTOize whenever new patches come in.

Address #105

Signed-off-by: Shane Peelar <[email protected]>
InBetweenNames added a commit that referenced this issue Oct 27, 2018
LTO patches are now applied directly from lto-overlay,
using functionality replicated directly from portage.
/etc/patches is now left untouched by LTOize, and
a script, 41-lto-patch.sh, is symlinked into your
portage bashrc.d.

You can experiment with your own LTO patches in /etc/patches
as usual.  Please consider submitting them upstream if you
make working ones.

Also, with this change, we no longer need to increment the version
number with LTOize whenever new patches come in.

Address #105

Signed-off-by: Shane Peelar <[email protected]>
@InBetweenNames
Copy link
Owner

OK -- master should have this addressed. It works the same way as the original /etc/patches mechanism, but sources directly from the overlay.

@mleise
Copy link
Author

mleise commented Oct 29, 2018

Thanks, that should do the trick!

@mleise mleise closed this as completed Oct 29, 2018
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

No branches or pull requests

2 participants