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
See the build failure there: https://nightly.geany.org/win32/build_win32_plugins_stderr.log
LGBI. The build error on Linux and GTK3 seems to only appear on Travis CI environment.
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.
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.
Strangely Geany builds with VTE for both GTK2 and GTK3, but its not mentioned in its `.travis.yml` so how does it get it?
@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*.
Geany doesn't depend on libvte at all
More worried about it not depending on `<vte/vte.h>`
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.
And `HAVE_VTE` actually means `WANT_VTE_AND_AM_GUESSING_THE_API` :grin:
LGBI. The build error on Linux and GTK3 seems to only appear on Travis CI environment.
Nah it's my bad, dddd4bcc0ee5c756e08b3cffa13bdf43cad2ba1c 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.
@b4n pushed 1 commit.
9b90cd9 travis: Update dependencies
@b4n pushed 2 commits.
959d0d9 utils: Add support for VTE 2.90 4f238af travis: Don't require packages that don't exist
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).
OK, so I have commits to check for VTE-2.90 as well, but it will need actual code support to be useful, so I took it out from this PR. If it's wanted, I can push it somewhere again.
…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.
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.
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/347a0c15fa1d67d3830c... :)
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.
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?
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?
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).
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/f0dbabab09f2ffc0f25054deb58ea7... or am I missing something?
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 `<vte/vte.h>` 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.
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.
I removed the ```#include <vte/vte.h>``` lines from ```scope.c``` and ```prefs.c``` and it still builds fine for GTK2 and 3 on Linux.
@LarsGit223 did you oush your changes regarding the `vte.h` include? Besides this, is here anything missing to be merged?
@eht16: no, I didn't. I already have a fork from the geany main repository and don't know how to fork a second one from b4n's repository.
Anyway, once this is merged I can update and deliver a PR with the changes.
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
@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.
@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 :)
@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.
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.
@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.
@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.
Merged #787 into master.
github-comments@lists.geany.org