GNOME Bugzilla – Bug 768707
Use upstream gettext instead intltool
Last modified: 2016-11-29 08:17:52 UTC
Patch attached
Created attachment 331287 [details] [review] Use autoreconf instead custom script
Created attachment 331288 [details] [review] Use upstream gettext
Looks good to me, fwiw
(In reply to Matthias Clasen from comment #3) > Looks good to me, fwiw Thanks for a comment, I don't have any experience with intltool/gettext. I am going to test the patches at least...
Review of attachment 331287 [details] [review]: Thanks for the patches, it seems it works properly, however INSTALL file is filled by some generic content after building, this should not happen. GVfs uses empty INSTALL file, so we may fill it with some generic content probably, but then it should be part of the commit... Also please add a link to this bug into the commit description.
Review of attachment 331288 [details] [review]: Several untracked files are added, so they should be added in .gitignore probably. Also it seems that gettext files are not updated, when I change some .po file, this is a problem probably.
Created attachment 331384 [details] [review] po: Support "Language" header field This fixes the following warnings printed by gettext: warning: header field 'Language' missing in header warning: header field 'Language' still has the initial default value
Comment on attachment 331384 [details] [review] po: Support "Language" header field This patch is obsoleted by commit ed27604.
Created attachment 332630 [details] [review] build: Use autoreconf instead custom script I tried to fix the issues mentioned in the reviews...
Created attachment 332631 [details] [review] build: Use upstream gettext instead intltool ...however polkit policy file has been added in the meantime and it is not buildable currently. So I tried to fix it using the following change: -@INTLTOOL_POLICY_RULE@ +org.gtk.vfs.file-operations.policy: org.gtk.vfs.file-operations.policy.in Makefile + $(AM_V_GEN) $(MSGFMT) --xml --template $< -d $(top_srcdir)/po -o $@ Unfortunatelly, it fails now with: /usr/bin/msgfmt: cannot locate ITS rules for org.gtk.vfs.file-operations.policy.in Don't you have an idea how to fix it? It seems to me that custom ITS file should be added for policy files, however it should be added by polkit, not by GVfs, am I right?
Ok, I will answer myself, it has been added recently: https://cgit.freedesktop.org/polkit/commit/?id=c788192 I will retest the patches later with it...
its rules for policy files have been merged here: https://cgit.freedesktop.org/polkit/commit/?id=c78819245ff8a270f97c9f800773e727918be838 Until that is in a release and widely available, I suggest that you could perhaps copy the file in your tree, and direct gettext to use those copies ?
Maybe I am silly, but I don't see something like --its option for msgfmt. its/loc files must be installed in the $(datadir)/gettext/its as per the docs. So I can't use msgfmt during build if the its/loc files are not yet installed... can I? What I need to do in order to replace @INTLTOOL_POLICY_RULE@? I am ok to wait with gettext migration until the its/loc files are distributed by polkit itself, or should we do it in this cycle from some reason?
See the last section in https://blogs.gnome.org/mclasen/2016/07/21/using-modern-gettext/
I saw your article. I am not sure I understand correctly your Comment 12. Did you mean to distribute loc/its files in gvfs tree before new gettext is available, or just use those files on my system for testing. You can skip the following and forget on Comment 13 if you meant the latter... My problem is that I can't simply add the following into gvfs, because polkit.loc and polkit.its files are installed during "make install"...: +itsdir = $(datadir)/gettext/its +its_DATA = polkit.loc polkit.its +EXTRA_DIST += $(its_DATA) ...but the following code is executed during "make", so the files are not yet installed and msgfmt fails... +org.gtk.vfs.file-operations.policy: org.gtk.vfs.file-operations.policy.in Makefile + $(AM_V_GEN) $(MSGFMT) --xml --template $< -d $(top_srcdir)/po -o $@ ...so I suppose that --its option for msgfmt should be added in order to solve such cases. Anyway won't be there a conflict once polkit is released?
Created attachment 332808 [details] [review] build: Use upstream gettext instead intltool The patch works properly with manually installed polkit.loc, polkit.its files. I added polkit requirement to 0.144 version which already contains those files. Let's wait for the required polkit release...
My idea was to have (uninstalled) copies of polkit.its/.loc in the gvfs source tree, in $(top_srcdir)/gettext/its/, and do $(AM_V_GEN) XDG_DATA_DIRS=$(top_srcdir) $(MSGFMT) --xml ... (note that I haven't actually tried this myself, but what I have tried is to add a workaround for the case that the polkit.its file is not found: %.policy: %.policy.in $(AM_V_GEN) $(MSGFMT) --xml -d $(top_srcdir)/po --template $< -o $@ || cp $< $@ This falls back to installing the untranslated policy file if msgfmt couldn't merge the translations (e.g. in the case that gettext is older than 0.19.8)
That's it, I haven't realized that I can specify custom XDG_DATA_DIRS...
Created attachment 332988 [details] [review] build: Use upstream gettext instead intltool Add || cp $< $@ fallback...
Created attachment 332989 [details] [review] build: Include its rules for polkit temporarily
Comment on attachment 332630 [details] [review] build: Use autoreconf instead custom script commit c641009dec6873aed9bbc00c74936263148b5d4c
Comment on attachment 332988 [details] [review] build: Use upstream gettext instead intltool commit 812f42e6bec9d9adf7c97d83fb55f0f0e4f3e3d6
Comment on attachment 332989 [details] [review] build: Include its rules for polkit temporarily commit c4da270a189001393c2eb2c5ee2ddfb3f429a944
this doesn't quite work. gettext looks in XDG_DATA_DIRS/gettext/its i think, so XDG_DATA_DIRS=$(top_srcdir) is right. (also should be $(srcdir) not $(top_srcdir) right ?) Backstory is whot tried gedit admin:///etc/shadow and didn't work <halfline> fenrig: instead of gedit /etc/shadow do gedit admin:///etc/shadow <halfline> shoot, missed him <whot> halfline: he may have just sudo gparted himself off irc :) <whot> (or she) <halfline> haha <linkmauve1> halfline, oh, is that gvfs or polkit? <halfline> linkmauve1: both actually! <linkmauve1> Sounds nice. <linkmauve1> But if it’s hidden like that I guess most users will miss it. <whot> halfline: action org.gtk.vfs.file-operations is not registered <SardemFF7> nice <halfline> yea there's an open bug about making it a built in feature in nautilus / gtk file browser <whot> halfline: what am I missing here? <halfline> whot: are you running f24 ? <whot> 25 <halfline> hmm one sec <whot> halfline: that's an update to f25, so maybe something didn't pull in correctly? <halfline> whot: do you have a file called /usr/libexec/gvfsd-admin ? <halfline> maybe it's f26 only * halfline checks <whot> halfline: yep, file is present and accounted for <halfline> whot: do you have /usr/share/polkit-1/actions/org.gtk.vfs.file-operations.policy ? <whot> halfline: yep <halfline> hmm <halfline> what does <halfline> $ pkaction -av org.gtk.vfs.file-operations <halfline> report ? <whot> No action with action id org.gtk.vfs.file-operations <halfline> okay so for some reason polkitd isn't looking in /usr/share/polkit-1/actions i guess <halfline> whot: what does ps -ef |grep polkit say ? <halfline> is it the one in /usr/lib ? <halfline> if you go to /proc/pid/exe for it does it show it as (deleted) ? <halfline> bbiab gotta take apart mizmo's laptop <whot> polkitd 937 1 0 Oct19 ? 00:00:04 /usr/lib/polkit-1/polkitd --no-debug <whot> and it's not deleted <halfline> only half typing because i'm using a screw driver, but try restarting it under strace and see if it opens the file <halfline> maybe try turning selinux off before doing that <halfline> whot: okay back <halfline> whot: do you have XDG_DATA_DIRS set to something ? <whot> halfline: so basic strace of the start shows a inotify is added to the actions directory, but nothing in there is openeed <whot> halfline: but that's just running sudo strace polkitd <whot> which fails, because the other one is running still (not sure how to work around that) <halfline> oh you probably have to pass --replace <whot> *facepalm* <whot> halfline: file is opend successfully <whot> no errors I can spot in the strace <halfline> okay do it again without strace and see if the debug output is helpful <whot> XDG_DATA_DIRS is system-set afaict, some flatpack and /usr/local/share:/usr/share <halfline> hmm <whot> and the debug output is pretty, but doesn't complain about anything either :) <whot> and I don't think I have ever done any polkit local configuration <whot> oh, wait <whot> ** (polkitd:26286): WARNING **: Unknown PolkitImplicitAuthorization string 'ne' <whot> afacit all the "ne" strings are prefixed with xml:lang="sr@latin" <whot> halfline: ok, found it <halfline> whot: what's the problem ? <whot> halfline: i removed the sr@latin lines, then it complained about the <allow_any xml:lang="sr">не</allow_any> <whot> then I removed all "xml:lang=" lines and it works <halfline> yea it's weird that that would be translated <halfline> this is in /usr/share/polkit-1/actions/org.gtk.vfs.file-operations.policy ? <whot> afaict it's happy with vi and sv, but unhappy with anything after sv <whot> yes, line 105 <whot> ok, it's unhappy with any string that's not "no" <whot> so it appears to not ignore all the xml:lang bits, or whatever it's supposed to do <halfline> so i don't think those should be there at all <halfline> my guess is either the wrong intltool rule is getting applied in the makefile <halfline> or maybe it got switched away from intltool to using gettext directly, but the conversion was wrong <halfline> clearly some of the strings in the file should be translated <halfline> but the implicit authorizations shouldn't <halfline> those map to an enum <whot> I'm gonna say that having 34 versions of "no" seems excessive :) <halfline> what version of gvfs do you have? * halfline might have already asked <halfline> no don't see it in scrollback <whot> gvfs-1.30.1-1.fc25.x86_64 <halfline> whot: so it does look like gvfs was switched from intltool to gettext <halfline> https://bugzilla.gnome.org/attachment.cgi?id=332989&action=diff <halfline> 4 <halfline> <its:translateRule selector="/action/description | <halfline> 5 <halfline> /action/message" <halfline> 6 <halfline> translate="yes"/> <halfline> looks plausibly right <halfline> only description and message <whot> afaict all tags are translated here <halfline> i don't actually know how the new gettext stuff works <whot> is that .its file actually used? <halfline> <locatingRule name="polkit policy" pattern="*.policy"> ← i wonder if that should be *.policy.in <halfline> whot: well the makefile does this: <halfline> $(AM_V_GEN) XDG_DATA_DIRS=$(top_srcdir) $(MSGFMT) --xml --template $< -d $(top_srcdir)/po -o $@ || cp $< $@ <halfline> so it's setting XDG_DATA_DIRS to $(top_srcdir) <halfline> i guess that should probably be $(srcdir) not $(top_srcdir) ? <halfline> (presumably it searches XDG_DATA_DRIS for .its files) <whot> and the .its file is in gettest/its, isn't it? <halfline> uh yea <halfline> so totally hosed <halfline> i guess we should reopen the bug
Created attachment 338058 [details] [review] 0001-gettext-switch-to-default-translate-no.patch see commit message, tldr: all tags are translated by default, we need to switch to a default of "no" and re-enable the ones we care about.
Review of attachment 338058 [details] [review]: Peter, thanks for the patch! I wonder why it translates everything by default... The polkit.its is copy-and-pasted from polkit upstream. It should be fixed in polkit upstream first, can you please prepare patch for them also? https://cgit.freedesktop.org/polkit/tree/data/polkit.its ::: gettext/its/polkit.its @@ +2,3 @@ <its:rules xmlns:its="http://www.w3.org/2005/11/its" version="2.0"> + <its:translateRule selector="//*" translate="no"/> It should be rather something like /policyconfig in order to avoid translation of <vendor>,<vendor_url> also...
(In reply to Ondrej Holy from comment #26) > It should be rather something like /policyconfig in order to avoid > translation of <vendor>,<vendor_url> also... //* selects everything, right? with this patch in place I only get description and message translated, the rest is ignored. so /policyconfig makes sense to be more precise but does this gain us anything? it's the root tag anyway.
polkit patch filed here: https://bugs.freedesktop.org/show_bug.cgi?id=98366 fwiw, this seems to be a side-effect of gettext vs intltool-merge
(In reply to Peter Hutterer from comment #27) > (In reply to Ondrej Holy from comment #26) > > It should be rather something like /policyconfig in order to avoid > > translation of <vendor>,<vendor_url> also... > > //* selects everything, right? with this patch in place I only get > description and message translated, the rest is ignored. so /policyconfig > makes sense to be more precise but does this gain us anything? it's the root > tag anyway. Yes, it selects everything, my fault. It is a bit more generic than my suggestion. However, probably all .its files on my system have a rule with root tag, that's why I suggested it, but I am ok with your patch... Thanks for filling this for polkit. I will push your patch once it is pushed to polkit.
It wasn't pushed to polkit upstream yet, however, I am pushing it for gnome-3-22 before the upcoming release...
commit 73bcd6c43f3dbdaf22f9e4bfcd7b97ab19a8f728 (gnome-3-22) commit 998d001d95fa1488abc54988a48bf84c29627d97 (master)
I ran into this on Fedora Rawhide testing the admin:/// thing; as I've documented that as a workaround for 'cannot run gedit as root on Wayland' at https://fedoraproject.org/wiki/Common_F25_bugs#wayland-root-apps , it'd be great if we could make sure this is fixed for F25...
aha, it looks like this was fixed before 1.30.2, which is in updates-testing for f25, so it'll be fixed when https://bodhi.fedoraproject.org/updates/FEDORA-2016-5522a26f9b is pushed stable. Rawhide only has 1.31.1, so we should really build 1.31.2 for Rawhide so this is fixed there (this was fixed between 1.31.1 and 1.31.2 on master).
We should not probably deal with Fedora tasks here, however, thanks that you pointed out that we should make new rawhide built, halfline made it already: http://koji.fedoraproject.org/koji/packageinfo?packageID=5487 So, I helped with my feedback for the F25 update at least...