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

geanypy: Update bundled ax_python_devel.m4 to latest upstream #339

Merged

Conversation

b4n
Copy link
Member

@b4n b4n commented Jan 18, 2016

This actually fixes building on Debian Unstable that ships
ax_python_devel.m4 in autoconf-archive, and apparently aclocal prefers
system directories. So we get the system version, which happens to be
the latest, which changed API.

To avoid breaking older systems that would also ship ax_python_devel.m4
but in an older version with the older API, keep using the old API too.

This actually fixes building on Debian Unstable that ships
ax_python_devel.m4 in autoconf-archive, and apparently aclocal prefers
system directories.  So we get the system version, which happens to be
the latest, which changed API.

To avoid breaking older systems that would also ship ax_python_devel.m4
but in an older version with the older API, keep using the old API too.
@b4n
Copy link
Member Author

b4n commented Jan 18, 2016

This however raises a more important concern: if the API changes again having the macro bundled doesn't help at least on Debian if the system has it too. I didn't yet investigate this part of the issue, but it might be something to worry about as apparently they sometimes change API.

BTW, although the incompatible change itself makes sense, maybe it'd be worth reporting the annoyance upstream? or to Debian?

@sardemff7 opinion? knowledge on the issue?

@b4n
Copy link
Member Author

b4n commented Jan 18, 2016

Also, maybe I we should rename ax_python_library.m4 as it effectively is a custom thing. Or submit it upstream, maybe.

@sardemff7
Copy link
Contributor

As I see it, the Makefile.am changes are backward-compatible. Thus I am not sure where the problem lies?

A few comments:

  • You should not bundle (other projects) m4 files in Git; these files are considered as development files and should be included in the relevant -dev(el) packages in your favorite distro.
  • If you really must trick aclocal, you can increase the #serial line so that it always picks the one in the repo. But you should not.
  • Do you really want Python 2 support? If not, I think Python 3 is distributed with a pkg-config file which you should definitely use. Maybe add a ./configure --with-python={2|3} switch?

@b4n
Copy link
Member Author

b4n commented Jan 18, 2016

As I see it, the Makefile.am changes are backward-compatible. Thus I am not sure where the problem lies?

It currently doesn't, I was just referring to possible future occurrences of a similar issue.

A few comments:

  • You should not bundle (other projects) m4 files in Git; these files are considered as development files and should be included in the relevant -dev(el) packages in your favorite distro.

Well. There's 2 problems here

  1. can we rely on every distros having autoconf-archive packaged, and can we trust it will contain ax_python_devel.m4?
  2. this would be a build dependency (from Git) no matter whether you want geanypy or not.
    • well, OK, we could use m4_ifdef() to conditionally use that. But it would be one more possible reason why the plugin is not available, and a lot less straightforward than a normal build dep.
  • If you really must trick aclocal, you can increase the #serial line so that it always picks the one in the repo. But you should not.

Oh, so it looks at the serial? I learned something :)

  • Do you really want Python 2 support? If not, I think Python 3 is distributed with a pkg-config file which you should definitely use. Maybe add a ./configure --with-python={2|3} switch?

Hum, maybe. In Debian there's also a pkg-config file for python2.7. Though, I don't think either one have the same information as the ones fetched by ax_python_devel -- but maybe they are just fine.

@b4n b4n added this to the 1.27 milestone Jan 18, 2016
@kugel-
Copy link
Member

kugel- commented Jan 18, 2016

geanypy is pretty much python2-only at the moment (as well as gtk2-only). I don't think any of the people involved is planning on changing that. So the short term fix is to ensure python2 is enforced for autotools.

@b4n b4n merged commit abf4249 into geany:master Jan 25, 2016
b4n added a commit that referenced this pull request Jan 25, 2016
…oconf-archive

geanypy: Update bundled ax_python_devel.m4 to latest upstream
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