This is a RFC regarding updating to the current version of Scintilla and getting off of the "fork" of the versions 3.x series (LongTerm3 branch).
The longer we wait to update, especially with the splitting off of lexers from Scintilla itself, we're basically just accumulating technical debt. Eventually the kind fellow currently maintaining the LongTerm3 branch will stop.
As far as I know, the main platforms Geany tries to support all have modern enough compilers/C++ standard libraries by now. There may be some older legacy/LTS platforms with users who won't be able to self-update to latest Geany without more effort, but with such limited developer time/interest/contributions for Geany, I don't think it's unreasonable to make some trade-offs like this in order to ensure Geany doesn't bitrot.
I might be willing to have a look at upgrading the code to work with latest Scintilla, but not if people are opposed, hence this RFC.
Agreed, its gotta happen, and the sooner the better.
Since its gonna be a disruptive change it should be made in a branch first, say `new_scintilla` then when its ready, mainline becomes LTS for anyone who wants to support it (I suspect nobody) and the `new_scintilla` branch becomes mainline.
If you want to start it, you can make a branch, so knock yourself out :grin:
Let's go for it. As @elextr said, at some point we need to switch and it won't get easier.
If I read https://en.wikipedia.org/wiki/C%2B%2B14 correctly, C++14 is supported since gcc 5 and this one was released five years ago. I think it is reasonable to require a compiler from 2015. Users who compile Geany themselves and use systems which are older than this, have to stick also with an older version of Geany.
I'm not completely sure what @elextr means by "mainline becomes LTS". I would not vote for maintaining a LTS release.
...C++14 is supported since gcc 5 and this one was released five years ago.
I think it is reasonable to require a compiler from 2015...
I believe Scintilla is using C++17 features, so probably more like 2016-2018 for GCC, depending on which features Scintilla uses.
Users who compile Geany themselves and use systems which are older than this, have to stick also with an older version of Geany.
Not necessarily. They can still use the latest greatest Geany, they just have to install an extra toolchain/stdlib in addition to the one that their distro ships with. It's more effort, but it's not exactly rocket science.
I would not vote for maintaining a LTS release.
:+1:
I'm not completely sure what @elextr means by "mainline becomes LTS".
Sorry to use proper terminology not shortcut (well _I_ knew what I meant :) I just meant branching the LTS from mainline just before merging the new_scintilla branch back in.
Since its gonna be a disruptive change...
I don't think it will be that disruptive, or perhaps I misunderstand you.
...it should be made in a branch first
Almost every PR is made in a separate branch.
If you mean having two parallel branches going forward (similar to Scintilla head and LongTerm3 branches), I disagree. If someone comes along and wants to maintain a legacy branch, they can very easily get the previous version from Git (ie. check out the last release tag), backport whatever patches, and even request to host it on the main Github Geany repo, which I doubt anyone would dispute. Until then, I don't think it's worth the effort and we don't really have the manpower to backport patches and stuff.
Almost every PR is made in a separate branch.
Ahh, I'm not doing very well am I, I meant a branch in the main repository, not a branch in your personal fork like most are, that way any Geany devs can contribute.
I don't think it will be that disruptive, or perhaps I misunderstand you.
If I remember correctly there were various changes that we had noted were likely to require more changes in Geany, IIRC things like the change of rectangular selections to multiple selections. There might be issues recording some of them, your searching may work better than mine :)
Also the change of the build system to make lexilla separate.
And any changes of scintilla.iface that mean lexers have more/less syntactic entities we need to map in highlightingmappings.h.
I believe Scintilla is using C++17 features, so probably more like 2016-2018 for GCC, depending on which features Scintilla uses.
Yeah, from a recent Scintilla release:
``` A C++17 compiler is required to build Scintilla. Microsoft Visual C++ 2019, GCC 7.3, Clang 4.0, Xcode 9.2 Clang or newer will work. Some slightly older compilers may still work. ```
Ubuntu 18.04 LTS uses gcc 7.5.0 AFAICT so should be fine.
If I remember correctly there were various changes that we had noted were likely to require more changes in Geany, IIRC things like the change of rectangular selections to multiple selections.
I haven't paid close attention or looked lately, but I believe the LongTerm3 branch of Scintilla that we're currently using has most/all of the applicable changes backported from Scintilla head.
Also the change of the build system to make lexilla separate.
Between the lexer library and the compiler change, I suspect the build system will account for most of the needed changes, yeah.
...C++14 is supported since gcc 5 and this one was released five years ago. I think it is reasonable to require a compiler from 2015...
I believe Scintilla is using C++17 features, so probably more like 2016-2018 for GCC, depending on which features Scintilla uses.
Sorry the misinformation. I refered to the note on the website. But yes, as @elextr said, the release announcements clearly state C++17. Even if gcc 7.3 from 2018 not as long ago as gcc 5, the other arguments are still true: the longer we wait, the harder the migration will be.
With some discussion of Scintilla 5.0 on `scintilla-interest` mailing list, I'm wondering if it might be worthwhile to just hold off until it's released, and then jump straight from 3 to 5. If/when I do this, I'm probably going to dedicate a day or two on a weekend to port it. I'd be pretty annoyed if after all the effort we were still a major version out of date and someone has to deep dive into the porting efforts all over.
From #2506
I have a local branch where I merged scintilla 4. The merge was pretty much a no brainer and required no changes to Geany itself. Given that and that nobody knows for sure when 5 will be released, I think we should not skip 4 "just because" if there's no good technical reason. We can still import 5 when it comes, maybe even before the next Geany release.
@kugel- sounds sensible. I just didn't want to spend a bunch of time porting only to still be out of date. If you've already done it for 4, then no point in waiting for 5, as you said. Just make a PR that closes this issue and we can worry about separately when the time comes.
@kugel- IIRC there was a change that treated rectangular selections as multiple selections, have you tested rectangular selections still work right?
Not yet, will check that.
I pushed my wip branch to https://github.com/kugel-/geany/tree/scintilla4
Before creating a PR I would like to transition to git-subtree, so that future updates become a simple "git subtree merge" command (merging from a side branch that tracks unmodified scintilla, just importing the upstream tarball to git).
I checked rectangular selections and couldn't notice a difference (selecting and pasting works as before).
Before creating a PR I would like to transition to git-subtree, so that future updates become a simple "git subtree merge" command (merging from a side branch that tracks unmodified scintilla, just importing the upstream tarball to git).
Can you expand on how this will work since the Scintilla directory in Geany is not a direct copy of the raw Scintilla repository?
I checked rectangular selections and couldn't notice a difference (selecting and pasting works as before).
And typing into and deleting in and pasting into? In particular a thin vertical selection? The thing I was most concerned about when I read that change in Scintilla is if Geany operations that take account of a rectangular selection still work?
Actually it's even simpler than git-subtree. It becomes a simple merge.
On my fork, I pushed two branches, scintilla4 and scintilla-import.
- The latter contains the unmodified scintilla sources from tarball plus a script to mechanically import a new tarball. - scintilla4 contains the master branch + a merge of that scintilla-import branch
The catch is that the merge retained all relevant Geany modifications to scintilla, i.e. GEANY_API_SYMBOL, selected lexers and build system integration; but to git we merged in the vanilla scintilla code. Therefore, future merges of that branch will only require a simple merge of that scintilla-import branch (`git merge scintilla-import`) and all Geany modifications will be left intact (unless they create conflicts of course).
The merge is crucial so that files deleted in upstream scintilla get deleted on our side as well.
However, it does mean we have to carry a complete copy of the scintilla sources. But I think that can be tolerated.
Carrying a complete copy of Scintilla would bloat the repository size and might get relevant over time. Do you know and tried the already existing update script (https://github.com/geany/geany/blob/master/scripts/update-scintilla.sh)? It also just works on an extract tarball and copies relevant files and patches our changes in.
I don't argue against the git-subtree solution, it sounds pretty good actually. I'm just concerned about the repository size and duplicating efforts which have already been done in the above mentioned script.
Yeah, I know the update script. I'm not concerned about the repository size t.b.h. geany is so tiny compared to everything else I usually work with and adding a few more files is not going to change that drastically. Git is pretty good at storing only actual changes.
I'm more worried about a) not detecting file deletions upstream and b) new stuff in files that we don't update. Also, in my eyes the existing script is more fragile than a plain `git merge` even though it has worked well in the past. In fact, the script I added just imports raw upstream scintilla into a side branch. Integrating that into master is not scripted because it's a plain merge.
But the most important selling point for me is that we can make changes directly to scintilla and have them automatically preserved when importing a new version without messing with a patch file.
I'll open a new PR for further discussion.
Closed #2519 as completed.
github-comments@lists.geany.org