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

Extend mingw support #374

Merged
merged 2 commits into from
Feb 27, 2016
Merged

Extend mingw support #374

merged 2 commits into from
Feb 27, 2016

Conversation

eht16
Copy link
Member

@eht16 eht16 commented Feb 27, 2016

Add support for compiling G-P using MSYS2/MINGW.

The MINGW check is copied shameless from Geany :).
The path fixups are those which we had for ages in the Waf
build system but never in Autotools.

With those changes, G-P actually start working on Windows.

On Windows we cannot use generated absolute paths based on the
installation prefix, so use relative paths instead.
frlan added a commit that referenced this pull request Feb 27, 2016
@frlan frlan merged commit c471626 into geany:master Feb 27, 2016
@@ -64,3 +64,20 @@ AC_DEFUN([GP_COMMIT_PLUGIN_STATUS],
test "$m4_tolower(AS_TR_SH(enable_$1))" = yes)
GP_STATUS_PLUGIN_ADD([$1], [$m4_tolower(AS_TR_SH(enable_$1))])
])

dnl GEANY_CHECK_MINGW
Copy link
Member

Choose a reason for hiding this comment

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

missing rename :)

@b4n
Copy link
Member

b4n commented Feb 27, 2016

Couldn't we simply use prefix=? seems awfully annoying to have fully different code paths here…

@eht16
Copy link
Member Author

eht16 commented Feb 27, 2016

I don't think so, at least prefix= would not be enough, examples:

  • datarootdir = ${prefix}/share -> this is the value in a generated Linux Makefile, this would lead to /share on Windows which is not relative, $datarootdir is used for LOCALEDIR and others
  • PKGLIBDIR is empty on Windows, on Linux it is $(libdir)/geany-plugins with $libdir being $prefix/lib, no idea why those are different and PKGLIBDIR seems unused at all to me

I'v just noticed DOCDIR is still broken, will fix this later today.

@b4n
Copy link
Member

b4n commented Feb 27, 2016

  • datarootdir = ${prefix}/share -> this is the value in a generated Linux Makefile, this would lead to /share on Windows which is not relative, $datarootdir is used for LOCALEDIR and others

Could be prefix=./ to make it relative.

  • PKGLIBDIR is empty on Windows, on Linux it is $(libdir)/geany-plugins with $libdir being $prefix/lib, no idea why those are different and PKGLIBDIR seems unused at all to me

But why have it different depending on the platform? Even if we used to, should we carry that on or couldn't we rely on it being UNIX-style, so on Windows it could be enough to use something like g_build_path(g_win32_get_package_installation_directory_of_module(), DATADIR, NULL) or alike?

@eht16
Copy link
Member Author

eht16 commented Feb 27, 2016

prefix=./ is a nice idea. However we need to set prefix to this, I tried to AC_SUBST prefix in configure.ac, not sure whether this will work. Though this will cause new problems because make install will then install the files into CWD, I guess. Using DESTDIR might workaround this but still I guess installing into CWD might be surprising.
Anyway, I'm just building Geany+G-P with prefix=./ to see whether it works at all.

About PKGLIBDIR: you are right. But currently my main focus is to get it all running at all. We are only two weeks from the release date and we don't have a working Geany, G-P, GTK bundle nor any installers yet. As usual, nobody started working on this stuff in time, including me :(.

Though I'm optimistic we've manage the release: the bundle scripts (for Geany and G-P) are pretty much ready, the installers hopefully need only small adjustments. Currently it is mostly Autotools which eat my time :(.

@b4n
Copy link
Member

b4n commented Feb 27, 2016

prefix=./ is a nice idea. However we need to set prefix to this, I tried to AC_SUBST prefix in configure.ac, not sure whether this will work. Though this will cause new problems because make install will then install the files into CWD, I guess. Using DESTDIR might workaround this but still I guess installing into CWD might be surprising.
Anyway, I'm just building Geany+G-P with prefix=./ to see whether it works at all.

I'm afraid using any relative directory will create mass confusion and problems, and that Autotools will refuse it anyway. I guess the "right" solution is --prefix=/ combined with DESTDIR=/where/to/install.
Installing in relative directories will be problematic from different Makefiles, as it'll be relative from different locations.

But currently my main focus is to get it all running at all. We are only two weeks from the release date and we don't have a working Geany, G-P, GTK bundle nor any installers yet. As usual, nobody started working on this stuff in time, including me :(.

Though I'm optimistic we've manage the release: the bundle scripts (for Geany and G-P) are pretty much ready, the installers hopefully need only small adjustments. Currently it is mostly Autotools which eat my time :(.

Yeah… that's the reason why I didn't merge the "remove Waf" for Geany yet, because I didn't actually get anything on the way for me to have a working Autotools Windows setup, and that I didn't feel like you guys were happy enough with it to just never use it again. So I'd rather have Waf one more cycle than not being able to make the release… although it's also a bad idea as with this way of thinking it won't go forward. So well, both are bad :]

@eht16
Copy link
Member Author

eht16 commented Feb 28, 2016

prefix=./ results in more problems than it solves. I got many weird issues with this :(. For G-P GeanyLua even refused to compiled, saying libtool: error: only absolute run-paths are allowed.

About Waf dropping: I'm working on this Windows+MSYS2+Autotools stuff since a few days quite hard and so I'm willing to get it working until the release, otherwise I could waste my time with more fun :).
More seriously, yes, I started again way too late with this in terms of close release date. But also hoped other people might be more interested in getting this to work, after the big "yeah, let's remove Waf" enthusiasm.

Another topic: can I push further changes to this topic into this already merged branch or should I better create a new branch (the branch just got merged too early)?

@b4n
Copy link
Member

b4n commented Feb 28, 2016

prefix=./ results in more problems than it solves. I got many weird issues with this :(. For G-P GeanyLua even refused to compiled, saying libtool: error: only absolute run-paths are allowed.

Yeah not surprising. Adgain I guess the "right" solution is --prefix=/ combined with DESTDIR=/where/to/install.

About Waf dropping: I'm working on this Windows+MSYS2+Autotools stuff since a few days quite hard and so I'm willing to get it working until the release, otherwise I could waste my time with more fun :).

Heh, I'm all for a working AT in the next release, I just know I'm not willing to spend the time needed for that myself right now :)

More seriously, yes, I started again way too late with this in terms of close release date. But also hoped other people might be more interested in getting this to work, after the big "yeah, let's remove Waf" enthusiasm.

Yeah unfortunately that happens :( And while I do like the idea, I also know I'm reluctant to mess with build stuff under Windows because I fear it. I think (and hope) I made clear I liked the idea but wouldn't really be of much help… anyway, I could still kick me off and give it a try, maybe it just works (for the Geany part).

Another topic: can I push further changes to this topic into this already merged branch or should I better create a new branch (the branch just got merged too early)?

You can, I just thing we won't ever be able to re-open it (which is mostly cosmetic somehow). You could also open a new PR for the same branch. And don't forget to add the "WIP" label so no one gets confused :)

@eht16 eht16 added the Windows label Feb 28, 2016
@eht16 eht16 self-assigned this Feb 28, 2016
@eht16 eht16 added this to the 1.27 milestone Feb 28, 2016
@eht16
Copy link
Member Author

eht16 commented Feb 29, 2016

prefix=./ results in more problems than it solves. I got many weird issues with this :(. For G-P GeanyLua even refused to compiled, saying libtool: error: only absolute run-paths are allowed.

Yeah not surprising. Adgain I guess the "right" solution is --prefix=/ combined with DESTDIR=/where/to/install.
Maybe. I did not used this but kept the previous approach which is a bit
more cumbersome but seems to work for now.

More seriously, yes, I started again way too late with this in terms of close release date. But also hoped other people might be more interested in getting this to work, after the big "yeah, let's remove Waf" enthusiasm.

Yeah unfortunately that happens :( And while I do like the idea, I also know I'm reluctant to mess with build stuff under Windows because I fear it. I think (and hope) I made clear I liked the idea but wouldn't really be of much help… anyway, I could still kick me off and give it a try, maybe it just works (for the Geany part).
No worries. You do tons of cool and more important stuff. Don't waste
your time with Windows issues.

Another topic: can I push further changes to this topic into this already merged branch or should I better create a new branch (the branch just got merged too early)?

You can, I just thing we won't ever be able to re-open it (which is mostly cosmetic somehow). You could also open a new PR for the same branch. And don't forget to add the "WIP" label so no one gets confused :)
Thanks. We got: #376

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

Successfully merging this pull request may close these issues.

3 participants