After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 764021 - Use __UCLIBC__ to select for uClibc features
Use __UCLIBC__ to select for uClibc features
Status: RESOLVED FIXED
Product: geocode-glib
Classification: Other
Component: general
unspecified
Other Mac OS
: Normal normal
: ---
Assigned To: geocode-glib maintainer(s)
geocode-glib maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2016-03-22 10:02 UTC by Anthony G. Basile
Modified: 2016-03-30 17:36 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
0001-geocode-glib.c-use-__UCLIBC__-to-check-for-uClibc-fe.patch (1.79 KB, patch)
2016-03-22 10:02 UTC, Anthony G. Basile
committed Details | Review

Description Anthony G. Basile 2016-03-22 10:02:33 UTC
Created attachment 324527 [details] [review]
0001-geocode-glib.c-use-__UCLIBC__-to-check-for-uClibc-fe.patch

Commit f0f85d8d introduces __GLIBC__ to check for glibc only features. However this is not sufficient for uClibc because it shares code with glibc.  To select for features in glibc but not uClibc, we need "defined(__GLIBC__) && !defined(__UCLIBC__)".

We note parenthatically that this is not required for musl which avoids
all such ugly marcos.
Comment 1 Anthony G. Basile 2016-03-22 10:07:01 UTC
Downstream bug reports:

geoclue:  https://bugs.freedesktop.org/show_bug.cgi?id=94521

gentoo:  https://bugs.gentoo.org/show_bug.cgi?id=577290
Comment 2 Zeeshan Ali 2016-03-22 19:30:37 UTC
(In reply to Anthony G. Basile from comment #0)
> Created attachment 324527 [details] [review]
> 0001-geocode-glib.c-use-__UCLIBC__-to-check-for-uClibc-fe.patch

Not sure which format is the patch in. Can you please create it with `git format-patch` or use `git-bz attach` to handle everything for you?
Comment 3 Anthony G. Basile 2016-03-22 20:39:55 UTC
(In reply to Zeeshan Ali (Khattak) from comment #2)
> (In reply to Anthony G. Basile from comment #0)
> > Created attachment 324527 [details] [review]
> > 0001-geocode-glib.c-use-__UCLIBC__-to-check-for-uClibc-fe.patch
> 
> Not sure which format is the patch in. Can you please create it with `git
> format-patch` or use `git-bz attach` to handle everything for you?

Wierd!!! I did create it with `git format-patch`.  Here are the steps to reproduce:

1. git clone https://github.com/GNOME/geocode-glib.git ; cd geocode-glib

2. <make the changes>

3. git add -A .

4. git commit

5. add message

6. git format-patch HEAD^

I also cloned https://git.gnome.org/browse/geocode-glib and applied my patch downloading directly from here using `git am` and no complaints.

Recreating it doesn't give me anything different except the SHA1.  I'll produce a pull request on github.  Even if you can't apply it there, at least you'll be sure that the patch was created in a sane way.
Comment 4 Anthony G. Basile 2016-03-22 20:50:10 UTC
(In reply to Anthony G. Basile from comment #3)
> Recreating it doesn't give me anything different except the SHA1.  I'll
> produce a pull request on github.  Even if you can't apply it there, at
> least you'll be sure that the patch was created in a sane way.

Wow first pull request ever against GNOME/geocode-glib on github:

   https://github.com/GNOME/geocode-glib/pull/1

If you want to apply it locally (ie maybe you can't merge it on github), just do

1. wget https://github.com/GNOME/geocode-glib/pull/1.patch

2. git am 1.patch 

I tested and it works at my end.
Comment 5 Jonas Danielsson 2016-03-23 08:54:30 UTC
Hi,

Thanks for the patch, could it be marked as a patch in here as well? So we can review it using bugzilla?
Comment 6 Jonas Danielsson 2016-03-23 08:55:00 UTC
(We could, I just did \o/)
Comment 7 Jonas Danielsson 2016-03-23 08:58:15 UTC
Review of attachment 324527 [details] [review]:

It looks fine, thanks. We do not use Signed-off-by's however. And I would prefer a short log of:
"Use __UCLIBC__ to check for uClibc features". And we can do without the parenthatically snarky comment :)

I can fix those details up while pushing this.

Btw, what is the fallout of this? And where are you trying to use geocode-glib?
Comment 8 Anthony G. Basile 2016-03-23 09:15:12 UTC
(In reply to Jonas Danielsson from comment #7)
> Review of attachment 324527 [details] [review] [review]:
> 
> It looks fine, thanks. We do not use Signed-off-by's however. And I would
> prefer a short log of:
> "Use __UCLIBC__ to check for uClibc features". And we can do without the
> parenthatically snarky comment :)
> 
> I can fix those details up while pushing this.
> 
> Btw, what is the fallout of this? And where are you trying to use
> geocode-glib?

1) thanks, please fix the details to fit your style

2) gentoo has a long tradition of targeting embedded systems on many different architectures.  we now build entire desktop systems using uclibc and musl libcs (see http://releases.freeharbor.net).  i hit this issue while building a xfce4 desktop on uclibc because of geoclue.  the idea is to extend the scope of "embedded" systems to include components like these.
Comment 9 Anthony G. Basile 2016-03-28 16:54:46 UTC
(In reply to Jonas Danielsson from comment #7)
> Review of attachment 324527 [details] [review] [review]:
> 
> It looks fine, thanks. We do not use Signed-off-by's however. And I would
> prefer a short log of:
> "Use __UCLIBC__ to check for uClibc features". And we can do without the
> parenthatically snarky comment :)
> 
> I can fix those details up while pushing this.
> 
> Btw, what is the fallout of this? And where are you trying to use
> geocode-glib?

Was this pushed?