Re: [Github-comments] [geany/geany-plugins] Scope: fix Automake conditional when building for Windows (#787)
Merged #787 into master. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/787#event-1988745172
Re: [Github-comments] [geany/geany-plugins] Scope: fix Automake conditional when building for Windows (#787)
@b4n, @elextr, @codebrainz: ok, I took the simplest way and just created a PR for the changes of the includes as the change is not depending on b4n's changes (see #793). @b4n, @codebrainz: sorry. I tried fetching b4n's repo but it seems to fetch everything out of it. Merging into one of my branches was not a problem. But I was afraid to push back in my github repo cause I feared that it would push all new branches into it. So I took the easy/save way. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/787#issuecomment-441756775
Re: [Github-comments] [geany/geany-plugins] Scope: fix Automake conditional when building for Windows (#787)
@LarsGit223 other solution is just show me the commit and I add it :) But @elextr and @codebrainz are right, you can easily just pull my changes into your Geany clone repo: you add my remote, and you fetch suff from it. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/787#issuecomment-441600608
Re: [Github-comments] [geany/geany-plugins] Scope: fix Automake conditional when building for Windows (#787)
Well _theres_ ya problem :) Stupid github. Guess all you can do is pull b4n's changes into your fork using Git and then publish a PR to G-P with those and your additional changes. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/787#issuecomment-441533249
Re: [Github-comments] [geany/geany-plugins] Scope: fix Automake conditional when building for Windows (#787)
@elextr: no, because I already have a fork from geany/geany-plugins. github just says _"You've already forked geany-plugins"_ if I click on fork in b4n/geany-plugins repo. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/787#issuecomment-441532134
Re: [Github-comments] [geany/geany-plugins] Scope: fix Automake conditional when building for Windows (#787)
@LarsGit223 can't you fork `b4n/geany-plugins` and make a pull request on it? Not that that is visible on this repository, thats one of the failings of github, its not transitive :) -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/787#issuecomment-441530588
Re: [Github-comments] [geany/geany-plugins] Scope: fix Automake conditional when building for Windows (#787)
@codebrainz: sorry, but how does that help me? I used ```git remote``` before e.g. to sync my fork of a repository with the original repo. I know I could locally clone b4n's repo on the CLI, but how do I get my changes back? I can't push of course because I don't have the permission. And a PR is a pure github function. So what would be the workflow? It is not clear to me from the git documentation. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/787#issuecomment-441528975
Re: [Github-comments] [geany/geany-plugins] Scope: fix Automake conditional when building for Windows (#787)
> I already have a fork from the geany main repository and don't know how to > fork a second one from b4n's repository. https://git-scm.com/book/en/v2/Git-Basics-Working-with-Remotes -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/787#issuecomment-441523755
Re: [Github-comments] [geany/geany-plugins] Scope: fix Automake conditional when building for Windows (#787)
@LarsGit223 did you oush your changes regarding the `vte.h` include? Besides this, is here anything missing to be merged? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/787#issuecomment-441442875
Re: [Github-comments] [geany/geany-plugins] Scope: fix Automake conditional when building for Windows (#787)
I removed the ```#include ``` lines from ```scope.c``` and ```prefs.c``` and it still builds fine for GTK2 and 3 on Linux. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/787#issuecomment-439146101
Re: [Github-comments] [geany/geany-plugins] Scope: fix Automake conditional when building for Windows (#787)
> After removing the "vte.h" include in scope.c it builds fine on Windows and > the include seems unnecessary to me after reading f0dbaba or am I missing > something? Yes, my fault. It's unused now. Maybe I needed it in some intermittent version of the code and forgot to remove it later. All calls to vte should be in ```conterm.c```. There also seems a unnecessary include in ```prefs.c```. Will re-check in the evening. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/787#issuecomment-438931125
Re: [Github-comments] [geany/geany-plugins] Scope: fix Automake conditional when building for Windows (#787)
> After removing the "vte.h" include in scope.c it builds fine on Windows and > the include seems unnecessary to me after reading f0dbaba or am I missing > something? Looks unused to me too. Note that the include of `` in `conterm.c` is guarded by `OS_UNIX` so if its needed in `scope.c` then it (and whatever uses it) should be guarded too. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/787#issuecomment-438853818
Re: [Github-comments] [geany/geany-plugins] Scope: fix Automake conditional when building for Windows (#787)
Just tested the changes on native Windows: `configure` runs fine but building Scope fails with `scope.c:28:10: fatal error: vte/vte.h: No such file or directory` :(. After removing the "vte.h" include in `scope.c` it builds fine on Windows and the include seems unnecessary to me after reading https://github.com/geany/geany-plugins/commit/f0dbabab09f2ffc0f25054deb58ea7f32b8256e9 or am I missing something? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/787#issuecomment-438849872
Re: [Github-comments] [geany/geany-plugins] Scope: fix Automake conditional when building for Windows (#787)
> could the Travis CI build be extended to also build for Windows Should be possible as we cross-compile also on Linux in the nightly builds setup. As usual, it just needs someone to port it to Travis. If someone wants to start, I can provide the necessary GTK stack (i.e. the files we use for the nightly builds). -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/787#issuecomment-438838305
Re: [Github-comments] [geany/geany-plugins] Scope: fix Automake conditional when building for Windows (#787)
> Look at 347a0c1 :) Saw it later, sorry :-) One question: could the Travis CI build be extended to also build for Windows to recognize Windows build related errors sooner? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/787#issuecomment-437074217
Re: [Github-comments] [geany/geany-plugins] Scope: fix Automake conditional when building for Windows (#787)
> But indeed it can be improved and made to fit in the future as consumers grow. Its the old thing with a library, do you build it and hope users come, or do you wait for users to demand then build it? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/787#issuecomment-436898987
Re: [Github-comments] [geany/geany-plugins] Scope: fix Automake conditional when building for Windows (#787)
> Also, regarding Vte I wanted to prevent that the utils lib always depends on > Vte even if no plugin is enabled which uses it. That's why there is this > "cross-reference" in the makefiles. I didn't know a better way to do it. If > there is a cleaner way for archiving this then I am open for suggestions. Look at https://github.com/geany/geany-plugins/pull/787/commits/347a0c15fa1d67d3830c3e3537c319d43c0b157e :) > Vte compatibility code might not be used often but there are definitely some > plugins which still need to be ported to Gtk3. […] I'm not saying that it's a bad thing to have VTE compatibility code in that utils lib, but that in the current state it doesn't seem super friendly for other users to me. But indeed it can be improved and made to fit in the future as consumers grow. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/787#issuecomment-436897080
Re: [Github-comments] [geany/geany-plugins] Scope: fix Automake conditional when building for Windows (#787)
> Presumably the idea was that since several plugins use the VTE library, > eventually in the future the other plugins' code/checks/compat could be > factored out into the common utils library. Correct. I have put some Gtk and Vte compatibility stuff in the common utils lib as there was no attempt for shared compatibility code yet. Also, regarding Vte I wanted to prevent that the utils lib always depends on Vte even if no plugin is enabled which uses it. That's why there is this "cross-reference" in the makefiles. I didn't know a better way to do it. If there is a cleaner way for archiving this then I am open for suggestions. Vte compatibility code might not be used often but there are definitely some plugins which still need to be ported to Gtk3. So I have made a first attempt to introduce some code in PR #750. There was a discussion about it some time ago but not much feedback IIRC. The bottom line is: this is a first attempt - suggestions/feedback/PRs are welcome. It won't be to much work adjusting the scope plugin to changes of the utils lib I think. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/787#issuecomment-436883617
Re: [Github-comments] [geany/geany-plugins] Scope: fix Automake conditional when building for Windows (#787)
> …which IMO is a very bad thing. BTW, as is I doubt the vtecompat thing in > utilslib makes much sense, because it seems highly specific to Scope's use of > it, but well. Presumably the idea was that since several plugins use the VTE library, eventually in the future the other plugins' code/checks/compat could be factored out into the common utils library. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/787#issuecomment-436845013
Re: [Github-comments] [geany/geany-plugins] Scope: fix Automake conditional when building for Windows (#787)
OK, as for Travis, it doesn't have libvte-2.91, but it does have libvte-2.90, which should likely be usable, maybe with a little bit of care (IIRC, 2.91 switched to using GdkRGBA instead of GdkColor). -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/787#issuecomment-436820456
Re: [Github-comments] [geany/geany-plugins] Scope: fix Automake conditional when building for Windows (#787)
@b4n pushed 2 commits. 959d0d9 utils: Add support for VTE 2.90 4f238af travis: Don't require packages that don't exist -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/geany/geany-plugins/pull/787/files/9b90cd94576bc61748aa3d1b322e47a0ab1373a4..4f238aff102ae2122c1017a4daf6e3421cbc18a4
Re: [Github-comments] [geany/geany-plugins] Scope: fix Automake conditional when building for Windows (#787)
@b4n pushed 1 commit. 9b90cd9 travis: Update dependencies -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/geany/geany-plugins/pull/787/files/347a0c15fa1d67d3830c3e3537c319d43c0b157e..9b90cd94576bc61748aa3d1b322e47a0ab1373a4
Re: [Github-comments] [geany/geany-plugins] Scope: fix Automake conditional when building for Windows (#787)
> LGBI. The build error on Linux and GTK3 seems to only appear on Travis CI > environment. Nah it's my bad, 4bcc0ee5c756e08b3cffa13bdf43cad2ba1c removes the check for whether Scope is enabled because I thought it was redundant but didn't realize this conditional was in utilslib… …which IMO is a very bad thing. BTW, as is I doubt the vtecompat thing in utilslib makes much sense, because it seems highly specific to Scope's use of it, but well. Anyway, the test should be in the corresponding module, so as it's in utilslib it should be in the utils checks. I did that in the latest version of this PR. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/787#issuecomment-436814153
Re: [Github-comments] [geany/geany-plugins] Scope: fix Automake conditional when building for Windows (#787)
And `HAVE_VTE` actually means `WANT_VTE_AND_AM_GUESSING_THE_API` :grin: -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/787#issuecomment-436792587
Re: [Github-comments] [geany/geany-plugins] Scope: fix Automake conditional when building for Windows (#787)
> Geany doesn't depend on libvte at all More worried about it not depending on `` Instead it silently depends on VTE devs not changing anything in the structs or consts, well, G* stuff has a __great__ record for that, though to be fair so far we seem to have gotten away with it. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/787#issuecomment-436792119
Re: [Github-comments] [geany/geany-plugins] Scope: fix Automake conditional when building for Windows (#787)
@elextr Geany doesn't depend on libvte at all, it dynamically loads it and has it's own copy of the prototypes and constants and such -- see *vte.c*. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/787#issuecomment-436782200
Re: [Github-comments] [geany/geany-plugins] Scope: fix Automake conditional when building for Windows (#787)
Strangely Geany builds with VTE for both GTK2 and GTK3, but its not mentioned in its `.travis.yml` so how does it get it? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/787#issuecomment-436780088
Re: [Github-comments] [geany/geany-plugins] Scope: fix Automake conditional when building for Windows (#787)
> The CI needs to be set up to get the GTK3 VTE library I think. Yes. I assume the entry "libvte-dev" in the config needs to be replaced with "libvte-2.91-dev". But I am not sure what that will do to other plugins. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/787#issuecomment-436773438
Re: [Github-comments] [geany/geany-plugins] Scope: fix Automake conditional when building for Windows (#787)
> The build error on Linux and GTK3 seems to only appear on Travis CI > environment. The CI needs to be set up to get the GTK3 VTE library I think. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/787#issuecomment-436757832
Re: [Github-comments] [geany/geany-plugins] Scope: fix Automake conditional when building for Windows (#787)
LGBI. The build error on Linux and GTK3 seems to only appear on Travis CI environment. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/787#issuecomment-436750922
Re: [Github-comments] [geany/geany-plugins] Scope: fix Automake conditional when building for Windows (#787)
See the build failure there: https://nightly.geany.org/win32/build_win32_plugins_stderr.log -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/787#issuecomment-433884671
[Github-comments] [geany/geany-plugins] Scope: fix Automake conditional when building for Windows (#787)
It is not valid to place `AM_CONDITIONAL`s inside a shell condition, because Automake has to known its value in all cases. Automake is smart enough to detect it if used in a AutoShell conditional (like `AS_IF`/`AS_CASE`/...), but can't work any magic inside a pure shell conditional (if/case/...). So, move the `AM_CONDITIONAL` outside the conditional, as the value is already conditional anyway. This PR also cleans up the conditional a little bit by giving it a plugin-specific name, and removing irrelevant test fro the condition (whether or not the plugin is enabled has little to no relevance here, and the plugin won't be built anyway). You can view, comment on, or merge this pull request online at: https://github.com/geany/geany-plugins/pull/787 -- Commit Summary -- * scope: Don't place AM_CONDITIONAL inside a shell condition * scope: Use plugin-specific namespace for plugin-specific conditional * scope: Cleanup conditional test -- File Changes -- M build/scope.m4 (3) M utils/src/Makefile.am (2) -- Patch Links -- https://github.com/geany/geany-plugins/pull/787.patch https://github.com/geany/geany-plugins/pull/787.diff -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/787
