void-linux / void-packages

The Void source packages collection
https://voidlinux.org
Other
2.59k stars 2.16k forks source link

Issue with automake 1.16.4+ and python #33559

Closed newbluemoon closed 2 years ago

newbluemoon commented 3 years ago

automake 1.16.4 made some changes to python.m4 which leads to $PYTHON_PREFIX instead of $prefix ending up in pkg-config .pc files.

So far I have found two:

/usr/lib/pkgconfig/compizconfig-python.pc has a line pyexecdir=${PYTHON_EXEC_PREFIX}/lib/python3.10/site-packages

and

/usr/lib/pkgconfig/xcb-proto.pc has pythondir=${pc_sysrootdir}${PYTHON_PREFIX}/lib/python3.10/site-packages

xcb-proto is a dependency of polybar which fails to build right now because $PYTHON_PREFIX cannot be resolved.

There is a newer automake 1.16.5 where they again made some changes to python.m4, but the problem consists. I found following patch (against 1.16.5) solves it (it actually reverts the lines to what they were before the change):

--- a/m4/python.m4  2021-10-04 04:51:12.000000000 +0200
+++ b/m4/python.m4  2021-10-15 08:38:08.575081215 +0200
@@ -263,7 +263,7 @@
    case $am_cv_python_pythondir in
    $am_py_prefix*)
      am__strip_prefix=`echo "$am_py_prefix" | sed 's|.|.|g'`
-     am_cv_python_pythondir=`echo "$am_cv_python_pythondir" | sed "s,^$am__strip_prefix,\\${PYTHON_PREFIX},"`
+     am_cv_python_pythondir=`echo "$am_cv_python_pythondir" | sed "s,^$am__strip_prefix,${PYTHON_PREFIX},"`
      ;;
    *)
      case $am_py_prefix in
@@ -305,7 +305,7 @@
    case $am_cv_python_pyexecdir in
    $am_py_exec_prefix*)
      am__strip_prefix=`echo "$am_py_exec_prefix" | sed 's|.|.|g'`
-     am_cv_python_pyexecdir=`echo "$am_cv_python_pyexecdir" | sed "s,^$am__strip_prefix,\\${PYTHON_EXEC_PREFIX},"`
+     am_cv_python_pyexecdir=`echo "$am_cv_python_pyexecdir" | sed "s,^$am__strip_prefix,${PYTHON_EXEC_PREFIX},"`
      ;;
    *)
      case $am_py_exec_prefix in

But I’m not sure about any implications. I filed a bug report upstream: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=51225

For reference: https://git.savannah.gnu.org/cgit/automake.git/log/m4/python.m4 The problematic commits are the ones from 2021-05-18 and 2021-09-19.

ahesford commented 3 years ago

I'm not exactly sure what automake is trying to accomplish here. If you rebuild xcb-proto with your patched automake, which of the following lines do you see in /usr/lib/pkgconfig/xcb-proto.pc?

The latter is what currently appears in the xcb-proto package.

In polybar, what happens if you export PYTHON_PREFIX=/usr either outside of any function or in a pre_configure function? Will pkg-config to the right thing?

newbluemoon commented 3 years ago

With the patched automake xcb-proto.pc contains the line

pythondir=${pc_sysrootdir}${prefix}/lib/python3.10/site-packages

which is as it was with 1.16.2. But when you build it with Void’s current 1.16.4 it’s

pythondir=${pc_sysrootdir}${PYTHON_PREFIX}/lib/python3.10/site-packages

Exporting PYTHON_PREFIX in the polybar template either inside or outside pre_configure has no effect.

ahesford commented 3 years ago

Seems reasonable. PR a patch (maybe with an update if you want) and please note whether you get a response to your upstream report. We want to consider any upstream justification for the breaking change.

newbluemoon commented 3 years ago

Done. When I get a response to the bug report I will post it here.

newbluemoon commented 3 years ago

In 1.16.5 they tried to restore the old behaviour and added a new configure option to activate the new one. They might just have forgotten to remove the double backslash. But it’s not really clear to me, either. ;)

newbluemoon commented 3 years ago

So I got a response :)

On 2021-10-17 12:32 , Karl Berry wrote:
>      https://git.savannah.gnu.org/cgit/automake.git/commit/m4/python.m4?id=ed8daa069a6c8ed34f7856c42402ec46f305e670
> 
>      However, this change leads to pkg-config .pc files containing the
>      variable name $PYTHON_PREFIX, not its content
> 
> I admit I didn't test much explicitly, but my understanding was that the
> use of ${PYTHON_PREFIX} in various places instead of its value was
> intentional, to allow for overriding at make time.
> 
> Thus, whatever the process is for creating .pc files, I guess it
> may need to change. A previous setting in the Makefile.am should yield
> the value of PYTHON_PREFIX to be substituted.

The documentation mentions in several places that the variables set by 
Automake are only really designed to be used in the Makefiles that it 
generates. Using AC_SUBST to directly substitute them into other kinds 
of files comes with the risk of exactly this kind of thing happening.

You could equally complain that some variables you want to use in the 
.pc file are defined in terms of $prefix; except it just happens that 
most .pc files already contain a prefix variable, so it works.

As Karl alluded to, if you want the fully expanded path, the .pc file 
needs to be generated by make, not configure. If you don't do it that 
way, the way these Makefile variables are defined will change and break 
your non-Makefiles sometimes.

- Josh

It seems automake is doing the right thing and xcb-proto should create its .pc file differently. However, I guess this behaviour is hidden in other projects using autotools, too, and will be revealed one after another when they start using automake >=1.16.4.

Maybe a post-install hook could check for $PYTHON_PREFIX and print a warning or error out?

newbluemoon commented 3 years ago

Just now I saw that there is an approved, simple patch for xcb-proto to adjust it to the new automake behaviour already. With it polybar builds fine now. Will open a PR to patch xcb-proto.

ahesford commented 3 years ago

The ugliest part about this is that they decided to use variable names that look totally different from other variables in the pc files, which makes the intention harder to gather. Why not pyexecprefix and pyprefix To make it clear these should be set like others? For that matter, why does PYTHON_EXEC_PREFIX even exist? What does this provide that can't be discerned by pyexecdir?

At this point, drop the patch in this PR since they clearly don't want the old behavior. I think whatever this breaks should be fixed by attrition; adding an xbps-src hook to scan pc files will add extra time to package building for little benefit. Presumable other upstreams will fix this problem in due time, and we can patch what we identify in the meantime.

newbluemoon commented 3 years ago

Already removed the patch from #33591 ;)

Regarding the motivation why they changed it I found https://gitlab.freedesktop.org/xorg/proto/xcbproto/-/merge_requests/25#note_1061544

The change in automake was motivated by the fact that PYTHON_PREFIX is not necessarily the same as $prefix. It could be a subdirectory (as in framework installations) or somewhere completely different.

The next automake release [i.e. 1.16.5] will probably change the default PYTHON_PREFIX back to $prefix for the sake of backwards compatibility. But it's not correct to assume that the two should always be the same. With 1.16.4, you can use the --with-python_prefix configure option to make them the same if that's how you want to configure.

github-actions[bot] commented 2 years ago

Issues become stale 90 days after last activity and are closed 14 days after that. If this issue is still relevant bump it or assign it.