Bug 100733

The person maintaining both is Matthias, and he said many years ago that he's not interested in bringing the forks closer together.

(In reply to Bastien Nocera from comment #1)
> The person maintaining both is Matthias, and he said many years ago that
> he's not interested in bringing the forks closer together.

Why?

Right now we have three different repos to update (this one, glib, and WebKit) that are almost always out of sync, because contributors don't know about the other ones.

Is the upstream repo even used for anything? Why did you recently commit to it?

(In reply to Michael Catanzaro from comment #2)
> (In reply to Bastien Nocera from comment #1)
> > The person maintaining both is Matthias, and he said many years ago that
> > he's not interested in bringing the forks closer together.
> 
> Why?

Ask Matthias.

> Right now we have three different repos to update (this one, glib, and
> WebKit) that are almost always out of sync, because contributors don't know
> about the other ones.
> 
> Is the upstream repo even used for anything? Why did you recently commit to
> it?

I commit to it because it's necessary to keep the shared-mime-info test suite working.

I don't think the copy of xdgmime on fd.o is used by anybody, and I consider it unmaintained. If anybody wants to take it over, let me know. As for the copy in glib, I would appreciate if somebody wanted to fully integrate it in glib (ie give up the posix-only constraints that come from sharing this)

We're using the following GLib commits in WebKit's fork of xdgmime:

https://gitlab.gnome.org/GNOME/glib/commit/2542b6f60492fadfb21f2c4a29976c3494c7e6c5
https://gitlab.gnome.org/GNOME/glib/commit/83d34f630939aa13539e03895bef87da738c4127

Looking through the downstream glib changes, I suggest we also take:

https://gitlab.gnome.org/GNOME/glib/commit/be7f40185fb2ce884112c1f8a4b196ea65350466
https://gitlab.gnome.org/GNOME/glib/commit/eb7b796bd206c31e336c89fb828a8a343ffb34ba

Here's one from Chromium:


diff --git a/base/third_party/xdg_mime/xdgmime.c b/base/third_party/xdg_mime/xdgmime.c
index c7b16bb..6dc58c2 100644
--- a/base/third_party/xdg_mime/xdgmime.c
+++ b/base/third_party/xdg_mime/xdgmime.c
@@ -558,13 +558,13 @@ xdg_mime_get_mime_type_for_file (const char  *file_name,
   mime_type = _xdg_mime_magic_lookup_data (global_magic, data, bytes_read, NULL,
 					   mime_types, n);
 
-  free (data);
   fclose (file);
 
-  if (mime_type)
-    return mime_type;
+  if (!mime_type)
+    mime_type = _xdg_binary_or_text_fallback(data, bytes_read);
 
-  return _xdg_binary_or_text_fallback(data, bytes_read);
+  free (data);
+  return mime_type;
 }
 
 const char *


They have two more patches at https://chromium.googlesource.com/chromium/src/+log/master/base/third_party/xdg_mime.

git-bz is broken for bugs.freedesktop.org (fails to steal session cookie) so I'll be doing this manually....

Created attachment 140407 [details] [review]
Patch

Created attachment 140408 [details] [review]
Patch

Created attachment 140409 [details] [review]
Patch

Created attachment 140410 [details] [review]
Patch

Created attachment 140411 [details] [review]
Patch

Created attachment 140412 [details] [review]
Patch

Comment on attachment 140412 [details] [review]
Patch

Review of attachment 140412 [details] [review]:
-----------------------------------------------------------------

::: src/xdgmimecache.c
@@ +122,5 @@
>  
>    /* Open the file and map it into memory */
> +  do
> +    fd = open (file_name, O_RDONLY|_O_BINARY, 0);
> +  while (fd == -1 && errno == EINTR);

This definitely needs braces. I'm also not a fan of do {} while loops but I guess this would do.

All the patches look alright, but the commit messages definitely need more information about the upstream fixes. Some explanation about the code that was changed would be useful, rather than having to either reference another bug tracker, or figure out what the problem was in the first place by reverse-engineering the fix.

Feel free to attach a single file for all the patches, or send them to me via email git-bz is still giving you grief.

Comment on attachment 140407 [details] [review]
Patch

Pushed the .gitignore patch (hopefully I'm making the right one obsolete).

Created attachment 140657 [details] [review]
Patches

Here they are in one file, with improved commit messages, and braces around the do/while, and one more fix from WebKit that I had forgotten. However, git doesn't understand how to apply the patches when they're concatenated together in one file, so you'll probably have to manually separate them out again....

(In reply to Michael Catanzaro from comment #17)
> Created attachment 140657 [details] [review] [review]
> Patches
> 
> Here they are in one file, with improved commit messages, and braces around
> the do/while, and one more fix from WebKit that I had forgotten. However,
> git doesn't understand how to apply the patches when they're concatenated
> together in one file, so you'll probably have to manually separate them out
> again....

git am...

I've reworded the commit for the first patch slightly, removed periods at the end of URLs to make them clickable, changed the pointer arithmetic patch (it isn't illegal to do maths on void pointers, it's just that GCC defines this behaviour, other compilers will have undefined behaviour).

Thanks for the patches!

Thanks for handling this, Bastien.

(In reply to Bastien Nocera from comment #19)
> (it isn't illegal to do maths on void pointers, it's just that GCC defines
> this behaviour, other compilers will have undefined behaviour).

Anyway, GCC started warning about it with the build flags we use for WebKit. :)