Merge ~jbenden/ubuntu/+source/vim:ubuntu/xenial-updates into ~usd-import-team/ubuntu/+source/vim:ubuntu/xenial-updates

[Joseph Benden]

[Impact]

A severe regression has been found in the GTK 3 variant of Gvim.
Upon start-up, it crashes immediately with multiple failed
assertions.

A second related, but less severe problem may occur when opening
new tabs within the GTK 3 variant of Gvim.

[Test Case]

Please note that in the following test cases, a given machine may
experience either of the test cases, while being unable to
experience the other test case. Please do /not/ discount
the merits of this SRU on being able to only confirm a single
test case, but not both.

This patch equally corrects both test cases listed below, to
produce a working GTK 3 Gvim variant.

IMPORTANT: Both test cases below require the package "vim-gtk3"
to be installed, prior to testing for either test case's
impact to one's machine.

[[TEST CASE #1]]

1. From a terminal window, launch the GTK 3 variant of Gvim:

  $ vim.gtk3

2. Immediately upon launch (whereby the window may or may
   not briefly display); the terminal window will show error
   messages relating to `unity_gtk_menu_*` critical
   assertions.

   A full example of the experienced error output shown is:

```
(gvim:13519): GLib-CRITICAL **: g_ptr_array_insert: assertion 'index_ <= (gint)rarray->len' failed
** (gvim:13519): CRITICAL **: unity_gtk_menu_shell_get_item: assertion '0 <= index && index < items->len' failed
** (gvim:13519): CRITICAL **: unity_gtk_menu_item_get_child_shell: assertion 'UNITY_GTK_IS_MENU_ITEM (item)' failed
** (gvim:13519): CRITICAL **: unity_gtk_menu_shell_get_item: assertion '0 <= index && index < items->len' failed
** (gvim:13519): CRITICAL **: unity_gtk_menu_item_get_label: assertion 'UNITY_GTK_IS_MENU_ITEM (item)' failed
** (gvim:13519): CRITICAL **: unity_gtk_menu_item_get_icon: assertion 'UNITY_GTK_IS_MENU_ITEM (item)' failed
```

[[TEST CASE #2]]

1. From a terminal window, launch the GTK 3 variant of Gvim:

  $ vim.gtk3

2. Upon the window opening, attempt to open a new tab by entering
   the key sequences "<ESC>:tabnew" and pressing <ENTER>. The
   program should now SEGV and abort.

[Regression Potential]

* The bug is possibly applicable to all later Ubuntu releases,
  which combine Unity and GTK 3.

* The upstream Vim project has not incorporated the fix, and
  most likely will not. The specifics behind the fix might
  be construed as too specific to the combination of Ubuntu,
  Unity, and GTK 3; and not general enough to patch for all
  downstream distributions.

[Other Info]

* Further information about this bug is available at the URLs:
  https://github.com/vim/vim/issues/851
  https://bugs.launchpad.net/ubuntu/+source/vim/+bug/1650666

[Nish Aravamudan (nacc)]

$ git ubuntu lint --verbose jbenden/ubuntu/xenial-updates
08/25/2017 14:34:53 - DEBUG:Executing: git config gitubuntu.lpuser
08/25/2017 14:34:53 - DEBUG:Executing: grep -q "* -ident" /home/nacc/work/imports/vim/.git/info/attributes
08/25/2017 14:34:53 - DEBUG:Executing: grep -q "* -text" /home/nacc/work/imports/vim/.git/info/attributes
08/25/2017 14:34:53 - DEBUG:Executing: grep -q "* -eol" /home/nacc/work/imports/vim/.git/info/attributes
08/25/2017 14:34:53 - DEBUG:Executing: git config gitubuntu.lpuser
08/25/2017 14:34:53 - DEBUG:Executing: sh -c "dpkg-parsechangelog -l- -SSource"
08/25/2017 14:34:53 - DEBUG:Executing: sh -c "dpkg-parsechangelog -l- -SDistribution"
Verified that only new modifications to changelog are top-most additions
08/25/2017 14:35:19 - DEBUG:Executing: sh -c "dpkg-parsechangelog -l- -n1 -SVersion"
08/25/2017 14:35:19 - DEBUG:Executing: sh -c "dpkg-parsechangelog -l- -n1 -o1 -SVersion"
Verified that debian/changelog version is as expected
08/25/2017 14:35:19 - DEBUG:Executing: git checkout jbenden/ubuntu/xenial-updates
08/25/2017 14:35:20 - DEBUG:Executing: update-maintainer
08/25/2017 14:35:20 - DEBUG:Executing: git --work-tree . add -f -A
08/25/2017 14:35:20 - DEBUG:Executing: git --work-tree . write-tree
08/25/2017 14:35:20 - DEBUG:Executing: git clean -f -d
08/25/2017 14:35:20 - DEBUG:Executing: git reset --hard jbenden/ubuntu/xenial-updates
08/25/2017 14:35:20 - DEBUG:Executing: git checkout edf5ebbb332805cfc835b544c1a87d832cef9b91
Verified that update-maintainer has been run
All lint checks passed

[Nish Aravamudan (nacc)]

Hello Joseph,

Your changes seem good and pass our linting. Thank you very much!

I'm happy to sponsor your changes, but can you please first update the bug with the SRU template from https://wiki.ubuntu.com/StableReleaseUpdates ?

[Joseph Benden (jbenden)]

I tested with Artful and the problem does not occur, with the default desktop (Ubuntu-branded GNOME 3).

[Nish Aravamudan (nacc)]

Joseph,

Just an FYI, I meant for you to update the bug with the SRU template. I'll copy it over there.

[Nish Aravamudan (nacc)]

Joseph,

As part of the SRU, we need to know if a similar fix is needed in Artful, or if the issues are not reproducible there to begin with. Are you able to confirm this?

[Nish Aravamudan (nacc)]

review: Needs Information

[Nish Aravamudan (nacc)]

And finally, while not necessarily super important (because of the nature of how these git merges get sponsored), it is preferred to target the 'devel' branches. The other branches are there primarily for archaeology. In this case, it's fine, because xenial-updates and xenial-devel are at the same commit, but for your own sanity in the future, you'd want to target xenial-devel.

[Joseph Benden (jbenden)]

I do not believe that Artful needs a similar patch; due to Unity being replaced by GNOME 3.

[Nish Aravamudan (nacc)]

On Mon, Aug 28, 2017 at 3:37 PM, Joseph Benden <email address hidden> wrote:
> I do not believe that Artful needs a similar patch; due to Unity being replaced by GNOME 3.

Unfortunately the default (GNOME) is irrelevant. Unity is still
available (unity7 from unity) in 17.10 and it would be preferred that
we test that setup (in 17.10, unity is now in universe not main, but
it's still an Ubuntu package). Are you able to set up a VM with that
configuration by any chance? If not, I can try later this week.

[Joseph Benden (jbenden)]

Due to concerns with DE conflict, I am proposing a slight patch change.

The detection of the GTK version identifier is useful (as it fixes the problem); however, the DE may actually be GNOME, which means that the patch is currently non-sufficient because it will interfere with non-Unity DE's using GTK 3.

I am proposing to check the XDG_CURRENT_DESKTOP env var for being equal to `Unity` as a test for Unity running.

Does this sound more reasonable? In the meantime I will produce an updated patch to this merge request for analysis/review.

[Jeremy Bicha (jbicha)]

In Ubuntu 17.04, XDG_CURRENT_DESKTOP is set to something like Unity7:Unity so you'd want to check if it contains Unity not if it equals Unity. (If that's the approach you end up taking.)

[Jeremy Bicha (jbicha)]

Also, before fixing on 16.04 LTS, it needs to be fixed in any newer supported releases (like 17.04).

f7ac6ed... by Joseph Benden on 2017-08-30

Check for Unity DE

[Joseph Benden (jbenden)]

I have updated the patch to include Unity DE detection, thereby ensuring the patches side-effects are only applied when ran within the Unity DE.

I have tested these changes on Xubuntu 16.04 and Ubuntu 16.04.

[Joseph Benden (jbenden)]

I have proposed a merge in Artful at:

https://code.launchpad.net/~jbenden/ubuntu/+source/vim/+git/vim/+merge/330033

Unmerged commits

f7ac6ed... by Joseph Benden on 2017-08-30

Check for Unity DE

da4ef25... by Joseph Benden on 2017-08-24

Fix SEGV upon start-up and opening new tabs with GTK 3 & Unity

* Fix SEGV upon start-up and opening new tabs; when GTK 3 is in
  use along-side of Unity. (LP: #1650666)
  - debian/patches/ubuntu-vim-gh-issue-851.patch: When GTK 3
    compilation, adjust command-line parameters to account for
    Unity.

diff --git a/debian/changelog b/debian/changelog
index 2265535..a8e30ff 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,13 @@
+vim (2:7.4.1689-3ubuntu1.3) xenial; urgency=low
+
+  * Fix SEGV upon start-up and opening new tabs; when GTK 3 is in
+    use along-side of Unity. (LP: #1650666)
+    - debian/patches/ubuntu-vim-gh-issue-851.patch: When GTK 3
+      compilation, adjust command-line parameters to account for
+      Unity.
+
+ -- Joseph Benden <[e-mail]>  Thu, 24 Aug 2017 09:44:44 -0700
+
 vim (2:7.4.1689-3ubuntu1.2) xenial-security; urgency=medium
 
   * SECURITY UPDATE: arbitrary shell execution via modelines
diff --git a/debian/patches/series b/debian/patches/series
index 289acca..43bb2dd 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -9,3 +9,4 @@ ubuntu-grub-syntax.patch
 update-upstart-syntax.patch
 ubuntu-releases.patch
 CVE-2016-1248.patch
+ubuntu-vim-gh-issue-851.patch
diff --git a/debian/patches/ubuntu-vim-gh-issue-851.patch b/debian/patches/ubuntu-vim-gh-issue-851.patch
new file mode 100644
index 0000000..89d06c6
--- /dev/null
+++ b/debian/patches/ubuntu-vim-gh-issue-851.patch
@@ -0,0 +1,71 @@
+Description: Resolve GTK 3 segfaults at start-up
+ * src/gtk_main.c: Incorporate GTK 3 tests and handle Unity arguments.
+Author: Kazunobu Kuriyama
+Author: Bram Moolenaar <[e-mail]>
+Author: oymy
+Origin: other, https://github.com/vim/vim/issues/851
+Bug: https://github.com/vim/vim/issues/851
+Reviewed-by: Joseph Benden <[e-mail]>
+
+--- a/src/gui_gtk.c
++++ b/src/gui_gtk.c
+@@ -631,6 +631,32 @@ menu_item_new(vimmenu_T *menu, GtkWidget
+     gtk_widget_show_all(menu->id);
+ }
+ 
++    static int
++is_unity()
++{
++    char	*de = NULL;
++    size_t	len = 0;
++    int		retval = 0;
++
++    if (getenv("XDG_CURRENT_DESKTOP") != NULL) {
++        de = strdup(getenv("XDG_CURRENT_DESKTOP"));
++        if (de == NULL) return retval;
++
++        len = strlen(de);
++
++        for (size_t i = 0; i < len; i++) {
++            de[i] = tolower(de[i]);
++        }
++
++        if (strstr(de, "unity") != NULL)
++            retval = 1;
++
++        free(de);
++    }
++
++    return retval;
++}
++
+     void
+ gui_mch_add_menu(vimmenu_T *menu, int idx)
+ {
+@@ -652,9 +678,11 @@ gui_mch_add_menu(vimmenu_T *menu, int id
+     parent_widget = (parent != NULL) ? parent->submenu_id : gui.menubar;
+     menu_item_new(menu, parent_widget);
+ 
++# if !GTK_CHECK_VERSION(3,4,0)
+     /* since the tearoff should always appear first, increment idx */
+-    if (parent != NULL && !menu_is_popup(parent->name))
++    if (parent != NULL && !menu_is_popup(parent->name) && is_unity())
+ 	++idx;
++# endif
+ 
+     gtk_menu_shell_insert(GTK_MENU_SHELL(parent_widget), menu->id, idx);
+ 
+@@ -773,10 +801,12 @@ gui_mch_add_menu_item(vimmenu_T *menu, i
+ 	if (parent == NULL || parent->submenu_id == NULL)
+ 	    return;
+ 
++# if !GTK_CHECK_VERSION(3,4,0)
+ 	/* Make place for the possible tearoff handle item.  Not in the popup
+ 	 * menu, it doesn't have a tearoff item. */
+-	if (!menu_is_popup(parent->name))
++	if (!menu_is_popup(parent->name) && is_unity())
+ 	    ++idx;
++# endif
+ 
+ 	if (menu_is_separator(menu->name))
+ 	{