FFe: Update gnome-terminal to 3.24 and vte to 0.48

[Jeremy Bícha]

gnome-terminal and vte are still at their 3.20 (and 0.44) versions because they now hard-depend on pcre2, but the pcre2 MIR (LP: #1636666) has stalled because there is already one version of pcre in main and it would be a major task to convert all of main to use pcre2.

So I reverted 3 commits for gnome-terminal and 5 for vte to make that MIR less urgent. I am concerned about the long-term maintainability of this approach - the vte patch is over 1600 lines.

gnome-terminal 3.22 and vte 0.46 have been tested in the GNOME3 Staging PPA for yakkety and zesty for months. I am not aware of any issues from that. This is slightly different since I haven't tested those versions without pcre2 until this weekend.

The development cycle for gnome-terminal and vte has been fairly quiet so I believe it's safe to go ahead and update gnome-terminal to 3.23.90 and vte to 0.47.90.

https://git.gnome.org/browse/gnome-terminal/log
https://git.gnome.org/browse/vte/log

The packages are currently available for testing at

https://launchpad.net/~gnome3-team/+archive/ubuntu/gnome3-staging/+packages?field.series_filter=zesty

[Iain Lane (laney)]

On Mon, Feb 20, 2017 at 06:15:21PM -0000, Jeremy Bicha wrote:
> ** Changed in: vte2.91 (Ubuntu)
> Importance: Undecided => Low
>
> ** Description changed:
>
> gnome-terminal and vte are still at their 3.20 (and 0.44) versions
> because they now hard-depend on pcre2, but the pcre2 MIR (LP: #1636666)
> has stalled because there is already one version of pcre in main and it
> would be a major task to convert all of main to use pcre2.
>
> So I reverted 3 commits for gnome-terminal and 5 for vte to make that
> MIR less urgent. I am concerned about the long-term maintainability of
> this approach - the vte patch is over 1600 lines.
>
> gnome-terminal 3.22 and vte 0.46 have been tested in the GNOME3 Staging
> PPA for yakkety and zesty for months. I am not aware of any issues from
> that. This is slightly different since I haven't tested those versions
> without pcre2 until this weekend.
>
> The development cycle for gnome-terminal and vte has been fairly quiet
> so I believe it's safe to go ahead and update gnome-terminal to 3.23.90
> and vte to 0.47.90.

If there aren't many changes (other than porting to PCRE2), what's the
proposed upside of getting onto the new versions?

It seems risky and annoying to maintain such a patch going forward -
it's only going to become harder to keep it working as time goes on,
until we get to drop it.

--
Iain Lane [ <email address hidden> ]
Debian Developer [ <email address hidden> ]
Ubuntu Developer [ <email address hidden> ]

[Jeremy Bícha (jbicha)]

Isn't that the opposite of how Feature Freeze Exception requests go, that it should be more likely to be accepted if there are *not* many changes?

Yesterday, it was reported that Ubuntu will have GNOME 3.24. And the first thing a GNOME packager for another distro said was well it isn't really GNOME 3.24 if you don't have *all* of it.

Now my first choice is that the MIR for PCRE2 be approved and I've been disappointed in how bug 1636666 has been handled so far.

I got the impression I was being told to try to revert the PCRE2 stuff in vte/gnome-terminal (since trying to port everything in main to PCRE2 is an unreasonable request). So it would be frustrating after I do the work if the answer is both No to the MIR and No to doing the update with the reverts.

I believe the reverts are safe for zesty; my maintainability concerns are more about maybe a year from now when the code has drifted more.

[Iain Lane (laney)]

Oh come on, there's no hive mind conspiracy against you here, you know that. It's individual developers answering questions that are asked of them and giving you their own judgement. If you'd have asked me before starting this work then I would have given the same answer ("do it only if the other changes make it worthwhile [and maybe consider cherry-picking those first if possible]").

The concern is this

 - Big revert patch
 - Can never drop it as long as we don't move to PCRE 2
 - Gets harder to maintain over time
 - No clear path to moving to PCRE 2
 - Given that, does having the new releases bring features and/or fixes that mean we should take on these difficulties or is this just taking on an unknown amount of work indefinitely just to have a larger number in the version string?

That is a *question*, not an accusation.

Or are you saying that I shouldn't be allowing these considerations to weigh into Feature Freeze requests, since the net change of *this* upload will be small?

Maybe that's fair and if that's the case then it means I'm answering as a fellow desktop team member, not as someone in the release team. In that case, your two ways to get this upload (three if you count 'ignore me and upload anyway' as one) are

 - Provide an explanation that this upload is worth the above future difficulties
 - Show that the net change is small and convince another member of the release team to +1 this request

[Jeremy Bícha (jbicha)]

I don't like the revert idea either.

But like I pointed out almost 4 months ago, there are just 3 options, right?
1. Accept PCRE2 in main
2. Keep gnome-terminal at 3.20 and vte at 0.44 indefinitely
3. Revert the PCRE2 changes, which allows postponing a choice between the first two.

I don't think #2 is better for maintainability than #3 really (We've done that with other packages before and it means that Ubuntu will only get a handful of bugfixes. Long term, it means we'll have created a mostly dead fork and I'm not convinced we had a good enough reason to do that. #3 is a fork too but it won't be as dead as quickly.)

I really think #1 is the right answer here but if we go with #3, we can keep ignoring the MIR for several more months.

[Iain Lane (laney)]

On Tue, Feb 21, 2017 at 01:00:02PM -0000, Jeremy Bicha wrote:
> I don't like the revert idea either.
>
> But like I pointed out almost 4 months ago, there are just 3 options, right?
> 1. Accept PCRE2 in main
> 2. Keep gnome-terminal at 3.20 and vte at 0.44 indefinitely
> 3. Revert the PCRE2 changes, which allows postponing a choice between the first two.
>
> I don't think #2 is better for maintainability than #3 really (We've
> done that with other packages before and it means that Ubuntu will only
> get a handful of bugfixes. Long term, it means we'll have created a
> mostly dead fork and I'm not convinced we had a good enough reason to do
> that. #3 is a fork too but it won't be as dead as quickly.)
>
> I really think #1 is the right answer here but if we go with #3, we can
> keep ignoring the MIR for several more months.

I'm sure there will be some point at which there will be a compelling
reason to upgrade. I just don't think we're there yet, and I think we'll
be able to maintain 3.20 for another (9 month) release without getting
into a potentially difficult situation with a complicated revert.

There will also definitely be a point at which some other upstream
switches off the dead branch of PCRE, and we get to have this fight with
two programs on one side rather than one.

That is, if you revert, then the patches will have to be removed. If
we're on 3.20 then we'll have to upgrade. Either way, the situation we
will have in Zesty will not be around forever.

So I think I'm out of arguments. Sorry for not being convinced. If any
other release team member is reading this and wants to weigh in, feel
free.

BTW I think we (Ubuntu as a whole) should push more on the PCRE
transition in the next cycle in any event.

Cheers,

--
Iain Lane [ <email address hidden> ]
Debian Developer [ <email address hidden> ]
Ubuntu Developer [ <email address hidden> ]

[Stéphane Graber (stgraber)]

I'm not seeing a compelling reason to switch to 3.24 and the newer vte other than the version bump.

If your argument is that after this revert, both gnome-terminal and vte are effectively identical feature-wise as what we have in the archive right now, then you're not introducing or removing features, in which case you don't need a Feature Freeze Exception in the first place.

If this change does introduce/remove features to gnome-terminal and vte, please list them in a readable format (not an upstream git log/diff).

I think the pcre2 MIR could do with an API diff between pcre3 and pcre2. Your massive patch suggests that the two APIs are very much different and that it's not something we can do as a typical transition, making the need for both of them to live in main together much clearer.

[Jeremy Bícha (jbicha)]

Unfortunately, upstream for gnome-terminal and vte doesn't care to provide NEWS files.

I said that gnome-terminal and vte hadn't changed much in the 3.24 cycle but there were changes in the 3.22 (0.46) cycle
- Add systemd user service
- Add Detach Terminal action to tab context menu (this is important because the ability to drag a tab out of a window was dropped in a 3.20 bug fix update but because of string freeze, this wasn't added until 3.22)
- Add "gnome-terminal --preferences" to allow changing preferences if you somehow managed to break something in settings so that you can't launch gnome-terminal normally
- a crash fix in the gnome-shell search provider https://git.gnome.org/browse/gnome-terminal/commit/?id=777e66d
- Various appstream metadata updates

3.24, vte 0.48
- async spawning https://git.gnome.org/browse/gnome-terminal/commit/?id=d4e25a57
- There's this GNOME initiative to use "fancy quotes", even for command line output so some strings were changed for that.

Here's the new vte symbols for 0.46 and 0.48
https://developer.gnome.org/vte/unstable/api-index-0-46.html
https://developer.gnome.org/vte/unstable/api-index-0-48.html

 vte_pty_spawn_async@Base 0.47.90
 vte_pty_spawn_finish@Base 0.47.90
 vte_regex_error_get_type@Base 0.45.90
 vte_regex_error_quark@Base 0.45.90
 vte_regex_get_type@Base 0.45.90
 vte_regex_jit@Base 0.45.90
 vte_regex_new_for_match@Base 0.45.90
 vte_regex_new_for_search@Base 0.45.90
 vte_regex_ref@Base 0.45.90
 vte_regex_unref@Base 0.45.90
 vte_terminal_event_check_regex_simple@Base 0.45.90
 vte_terminal_match_add_regex@Base 0.45.90
 vte_terminal_search_get_regex@Base 0.45.90
 vte_terminal_search_set_regex@Base 0.45.90

[Launchpad Janitor (janitor)]

Status changed to 'Confirmed' because the bug affects multiple users.

[Fred (eldmannen+launchpad)]

This is how my GNOME Terminal 3.20.2 looks under Wayland.
It looks as expected under X though.

Notice the ugly broken shadow.

[Jeremy Bícha (jbicha)]

Fred, please file a different bug for that issue.

[Launchpad Janitor (janitor)]

This bug was fixed in the package vte2.91 - 0.48.2-0ubuntu2

---------------
vte2.91 (0.48.2-0ubuntu2) artful; urgency=medium

  * Don't depend or build-depend on libpcre2-dev

 -- Jeremy Bicha <email address hidden> Tue, 02 May 2017 14:28:14 -0400

[Launchpad Janitor (janitor)]

This bug was fixed in the package gnome-terminal - 3.24.1-0ubuntu2

---------------
gnome-terminal (3.24.1-0ubuntu2) artful; urgency=medium

  * Revert 01_onlyshowin.patch to use Unity instead of Unity7
    to fix test failure triggered by desktop-file-validate

 -- Jeremy Bicha <email address hidden> Wed, 03 May 2017 06:52:22 -0400

[Gerald Nunn (gnunn)]

I have an upstream bug report for tilix in Ubuntu 17.10 where the VTE is complaining about not supporting PCRE2 despite VTE reporting that it is version 0.48.2 which should support it.

https://github.com/gnunn1/tilix/issues/916

Is this caused by Ubuntu going with the #3 option and reverting the PCRE2 commits in VTE? If so this is going to cause issues with any VTE based terminal emulator that is relying on checking the version number to determine available VTE functionality.

Any plans on correcting this with a hot fix?

[Max Ehrlich (queuecumber)]

Just throwing my hat in the ring as absolutely no one of influence: I think reverting functionality that is understood to be in a particular version of a package is inviting breakage.

If VTE 0.48.x is understood to have PCRE2 support, as announced by the VTE developers, then applications that depend on it are free to assume they can use that functionality. If Ubuntu patches that out, then you are going to break code that makes such an assumption. What's going on with Tilix is a good example of this.

The best option here is #1, to add PCRE2 into main. Based on my reading of #1636666, it really isn't that big a deal, but if the maintainers are dead set against it, then do #2 leave gnome terminal and VTE at their lower version numbers until there is more impetus to switch. #3 is objectively the wrong decision.

[Sebastian Geiger (lanoxx)]

This is really a mess. If we as developers cannot trust a particular version of a library to contain the features advertised by the documentation then that is a big problem.

As a developer working on open source software in my spare time I have enough to do to keep up with the latest library changes and migrating my package to drop deprecated features. I don't want to waste my time trying to understand why a particular feature does not work as advertised.

[Ari (ari-reads)]

This is still breaking tilix functionality and polluting the logs.

[Jeremy Bícha (jbicha)]

Ari, if tilix is logging excessively, please report the issue to the tilix developers. The Ubuntu tilix patch is really minimal and shouldn't have that effect.

[Thorsten (thorstenr-42)]

ari, i already have created a bug report for the tilix log messages: https://bugs.launchpad.net/ubuntu/+source/tilix/+bug/1785885

[Thorsten (thorstenr-42)]

Hi everyone,

I don't know which is the right place to post this, but the discussion here seems to be the most appropriate for bringing this up.

I looked more into the tilix warnings. After all, the warnings seem to be caused by the vte lib and especially in the reverted parts.
Therefore, I downloaded the source with apt-get source libvte-2.91-0 and printed the arguments of line 1906 which triggers the warnings.

#include <stdio.h> (at the top of the file)
printf("arguments: %d %d %d\n", g_regex_get_compile_flags(gregex),
                                G_REGEX_MULTILINE,
                                g_regex_get_compile_flags(gregex) & G_REGEX_MULTILINE);

So I started tilix and looked at the output:

arguments: 1 2 0

(tilix:5596): Vte-WARNING **: 10:32:57.957: (../../src/vtegtk.cc:1905):int vte_terminal_match_add_gregex(VteTerminal*, GRegex*, GRegexMatchFlags): runtime check failed: (g_regex_get_compile_flags(gregex) & G_REGEX_MULTILINE)
warning should be printed
arguments: 1 2 0

(tilix:5596): Vte-WARNING **: 10:32:57.957: (../../src/vtegtk.cc:1905):int vte_terminal_match_add_gregex(VteTerminal*, GRegex*, GRegexMatchFlags): runtime check failed: (g_regex_get_compile_flags(gregex) & G_REGEX_MULTILINE)
warning should be printed
arguments: 1 2 0

(tilix:5596): Vte-WARNING **: 10:32:57.957: (../../src/vtegtk.cc:1905):int vte_terminal_match_add_gregex(VteTerminal*, GRegex*, GRegexMatchFlags): runtime check failed: (g_regex_get_compile_flags(gregex) & G_REGEX_MULTILINE)
warning should be printed
arguments: 1 2 0

(tilix:5596): Vte-WARNING **: 10:32:57.957: (../../src/vtegtk.cc:1905):int vte_terminal_match_add_gregex(VteTerminal*, GRegex*, GRegexMatchFlags): runtime check failed: (g_regex_get_compile_flags(gregex) & G_REGEX_MULTILINE)
warning should be printed
arguments: 1 2 0

(tilix:5596): Vte-WARNING **: 10:32:57.957: (../../src/vtegtk.cc:1905):int vte_terminal_match_add_gregex(VteTerminal*, GRegex*, GRegexMatchFlags): runtime check failed: (g_regex_get_compile_flags(gregex) & G_REGEX_MULTILINE)
warning should be printed
arguments: 1 2 0

(tilix:5596): Vte-WARNING **: 10:32:57.957: (../../src/vtegtk.cc:1905):int vte_terminal_match_add_gregex(VteTerminal*, GRegex*, GRegexMatchFlags): runtime check failed: (g_regex_get_compile_flags(gregex) & G_REGEX_MULTILINE)
warning should be printed

As you can see both arguments itself are true. But why is a "bitwise and" used in g_warn_if_fail()? Since the first is 1 and the second 2, this causes 00000010 & 00000001 -> 00000000 and the warning is triggered?

So what I did and propose is to replace the "bitwise and" with a "logical or". This causes that the warning is only triggered if one of both arguments is false. -> The tilix warning is gone and everything seems work for me!

This line is created by the ubuntu specific patch "revert-pcre2.patch" and in this file in line 1065

Just for letting you know which part of tilix calls this function:
In the constructor in the file terminal.d the function applyPreferences() is called. There in the switch statement
the case SETTINGS_ALL_CUSTOM_HYPERLINK_KEY is calling loadRegex() which calls the vte function vte_terminal_match_add_gregex 6 times and trig...

[Thorsten (thorstenr-42)]

i should be more precise with the file name... the specific line is in src/vtegtk.cc line 1906

[Egmont Koblinger (egmont-gmail)]

> As you can see both arguments itself are true.

The first two arguments you printed are bitmasks, it doesn't make sense to directly interpret them as boolean.

> But why is a "bitwise and" used in g_warn_if_fail()?

Because that's what it wants to do: warn if the MULTILINE flag isn't set, regardless of the other flags.

> So what I did and propose is to replace the "bitwise and" with a "logical or".

This is semantically absolutely incorrect. The second argument is a nonzero constant, logically OR-ing would always give logical TRUE, hence the "warn if fail" not executed.

You could just as well remove the entire line, that'd be okay.

---

Let's take one step back. Comment #17 says "This is still breaking tilix functionality and polluting the logs" but the tilix bug linked from #19 only talks about the warnings. Maybe I'm missing something, but is Tilix's actual behavior broken? If so, how exactly, and where's the tracking Ubuntu bug of that? Or are we only talking about annoying but harmless warnings?

[Thorsten (thorstenr-42)]

thanks a lot for the clarification, this is really helping me to understand that! I am just a noob who wants to learn something and wants to find the cause of a problem which annoys me. Thus, i invested some time to dig into the problem :)

So the cause is that the MULTILINE flag is just that not set during compilation?

tilix behavior is not broken and the hyperlink regex, which when added causes the warning, is working. The only problem is that it is spamming the logs with these warnings.

[Egmont Koblinger (egmont-gmail)]

> So the cause is that the MULTILINE flag is just that not set during compilation?

Compilation of the regex when running tilix, not compilation of the source code.

I have to admit I don't know what this regex is for, and whether required in unpatched vte or not. If tilix is warning-free with unpatched vte then probably Ubuntu should just simply remove this line that emits the warning. (Well, Ubuntu should really move pcre2 to main and drop this entire patch, but that's a different story.)

[Egmont Koblinger (egmont-gmail)]

Fix: "I have to admit I don't know what this regex *flag* is for..."

[Thorsten (thorstenr-42)]

tilix seems to be warning-free with unpatched vte. Someone has posted these warnings upstream: https://github.com/gnunn1/tilix/issues/1428 and the response was "Please open a bug with Ubuntu, since they patch vte to remove pcre2 regular expressions this issue is likely caused by that." a

[Stu (stu-axon)]

I just tried building tilix to see if I could add some feature and hit this :(

[Thorsten (thorstenr-42)]

you probably have to apply the ubuntu tilix "pcre2-version-update" patch