Hello. During fluxbox-1.3.6 build portage produces the following message: * Creating a menu file (may take a while) ... /var/tmp/portage/x11-wm/fluxbox-1.3.6/work/fluxbox-1.3.6/util/fluxbox-generate_menu: line 334: printf: `U': invalid format character The last line is repeated multiple times. Build itself completes fine. Please fix. Reproducible: Always
Created attachment 393738 [details] build.log
Created attachment 393740 [details] emerge --info fluxbox
I'm working on a solution for this problem. This problem is only triggered with fluxbox-generate_menu's `-is` and `-ds` flags active, which searches through *.desktop files and creates menu entries based on them (in addition to icon files). What's tripping it up is the format string after some applications like firefox, whose .desktop file has `firefox %U` as its "Exec" entry. The fault is on fluxbox-generate_menu for not escaping these percent signs before adding menu entries. No solution yet, but I wanted to reply to inform others that the problem has been isolated.
Quick update: Changing `entry_exec` to `execname` on line 334 gets rid of the printf errors and fixes the semantic issue of icon paths: for instance, the tool was originally giving Firefox's entry a qjoypad icon on my system. I've suggested said fix upstream and will stay in touch with them. Please test it and let me know if it works okay. I'll attach a quick patch.
Created attachment 393812 [details, diff] Fixes printf issue in fluxbox-generate_menu This one-liner bypasses the printf complaint and seems to fix an underlying semantic bug that was previously missed as well.
(In reply to Daniel Campbell from comment #5) > Created attachment 393812 [details, diff] [details, diff] > Fixes printf issue in fluxbox-generate_menu > > This one-liner bypasses the printf complaint and seems to fix an underlying > semantic bug that was previously missed as well. This patch unfortunately cannot be applied. Portage fails to find the appropriate file paths.
Created attachment 394042 [details] build.log
Created attachment 394044 [details] fluxbox-generate_menu-printf-fix.patch.out
Created attachment 394046 [details] fluxbox-1.3.6.ebuild Ebuild I am using.
Created attachment 394052 [details, diff] fluxbox-generate_menu-printf-fix.patch > This patch unfortunately cannot be applied. Portage fails to find the appropriate file paths. My apologies, I'm still new to patching, let alone patching in Portage! With the help of grknight @ #gentoo-dev-help, I fixed the patch (which is attached), tested it, and confirms it works when placed into the `files` subdir and `epatch`ed in the ebuild. I'll include a patched ebuild if necessary. Please let me know if it works for you. Sorry for the hiccup!
Comment on attachment 394052 [details, diff] fluxbox-generate_menu-printf-fix.patch Corrected patch's filename to reflect edited ebuild from Coacher.
(In reply to Daniel Campbell from comment #10) > My apologies, I'm still new to patching, let alone patching in Portage! No problems. IMHO the easiest way to generate patches is to use project's vcs repo clone. Output of vcs's `diff` command is a valid patch for portage. (In reply to Daniel Campbell from comment #11) > Comment on attachment 394052 [details, diff] [details, diff] > fluxbox-generate_menu-printf-fix.patch > > Corrected patch's filename to reflect edited ebuild from Coacher. I can confirm that this patch fixes the issue for me. Thanks.
So, the proposed patch fixes the problem. What's left to do before adding it to the tree?
(In reply to Coacher from comment #13) > So, the proposed patch fixes the problem. What's left to do before adding it > to the tree? not alot. ~/cvsPortage/gentoo-x86/x11-wm/fluxbox $ ebuild fluxbox-1.3.6-r1.ebuild clean install >>> Completed installing fluxbox-1.3.6-r1 into /mnt/gen2/TmpDir/portage/x11-wm/fluxbox-1.3.6-r1/image/ strip: x86_64-pc-linux-gnu-strip --strip-unneeded -R .comment -R .GCC.command.line -R .note.gnu.gold-version and a few more lines. ~/cvsPortage/gentoo-x86/x11-wm/fluxbox $ grep format /mnt/gen2/TmpDir/portage/x11-wm/fluxbox-1.3.6-r1/temp//build.log yields Try `/mnt/gen2/TmpDir/portage/x11-wm/fluxbox-1.3.6-r1/work/fluxbox-1.3.6/build-aux/missing --help' for more information proving the patch effective in dealing with "line 334: printf: `U': invalid format character" *fluxbox-1.3.6-r1 (24 Jan 2015) 24 Jan 2015; Ian Delaney <idella4@gentoo.org> +files/1.3.5-generate_menu-printf-fix.patch, +fluxbox-1.3.6-r1.ebuild: revbump; Add patch prepared by proxy maintainer, D. Campbell, from bug #536370 addressing format issue in icon paths, fixes said Bug Name change. Remember the naming is arbitrary. (It could have been named I_love_sausage_rolls and it would still work). I prefixed it with a version giving a handle on when this patch became pertinent. This patch is also used in -1.3.5 giving it pertinence to both. I deleted 'fluxbox-' (since we already know it's fluxbox), giving us more brevity, by all of 2 chars!!!!! rationale for revbump ought be quite self explanatory. Being new to this group I shall refrain from finalising it totally fixed at this point. (More review never hurts too much) One final point. This shouts out for being put to, and accepted by, upstream even if modified. URL: beckons a bug / pull request filed upstream. Ditto #537356
Created attachment 394852 [details, diff] 1.3.5-fbgm-printf-grep-fix.patch I spoke with upstream, and they pointed out that execname was replacing entry_exec in the output, but not the grep itself. This patch expands the fix to the surrounding, relevant lines. I have e-mailed it to upstream and it's been merged. It should be present in the upcoming 1.3.7 release. This new patch should be applied for 1.3.5 and 1.3.6. Sorry for the formerly incomplete patch.
Daniel; good 1. You've renamed the patch but that's fine. It's pretty long but you've followed the pattern I set so that's no prob. 2. So, the idea here is to substitute 1.3.5-generate_menu-printf-fix.patch with the updated 1.3.5-fbgm-printf-grep-fix.patch. Right? 3. This new patch should be applied for 1.3.5 and 1.3.6 which actually translates to This new patch should be applied for 1.3.5-r1 and 1.3.6-r1, right? 4. Remember "URL: beckons a bug / pull request filed upstream." By procedure 'we' normally enter a link in the field "URL:" just above "Whiteboard:" 5. By convention we also normally enter the whole link of the commit upstream in the patch itself. Just look up patches in portage and you'll find some, not all, that have a copy paste of a link in the first few lines. Most any security patch will have this. These lines are designed not to interfere with the patch itself. The lines are treated by diff as comments until it hits the line starting with --- fluxbox-1.3.6/util/fluxbox-generate_menu.in etc. 6. --- fluxbox-1.3.6/util/fluxbox-generate_menu.in 2015-01-25 04:12:01.527057876 -0800 +++ fluxbox-1.3.6-patch/util/fluxbox-generate_menu.in 2015-01-25 04:11:40.870295713 -0800 Curious this works. For me I always have to delete the ${P}, in this case fluxbox-1.3.6/, for the patch to take. Don't know what's going on here but for consistency, it can be reduced to --- util/fluxbox-generate_menu.in 2015-01-25 04:12:01.527057876 -0800 +++ util/fluxbox-generate_menu.in 2015-01-25 04:11:40.870295713 -0800 So, patching a patch is one way to do it but it could still do with having those minor tidies done. It's not a big effort to simply re-make the patch incorporating the above and add as an attachment, obsolete the one it obsoletes and I can substitute it in both ebuilds in one hit. However, replacing the whole thing with a patch from an upstream commit replaces the whole thing anyway. Both will 'work'.
1. Sorry about that. I'm new to this, so if there's a way to rename it to the old name, that's fine with me. 2. Yes, it obsoletes the older one, but I don't mind if the name is kept intact. 3. Yes, sorry I didn't make that more clear. 4. I can't seem to edit that since I'm just proxy-maint. All I have for a link to the accepted patch are these: http://git.fluxbox.org/fluxbox.git/commit/?id=fc245408d6975d0813cd4440e7089d987b54d42e and https://github.com/fluxbox/fluxbox/commit/fc245408d6975d0813cd4440e7089d987b54d42e 5. Do you want me to add one of the above two links to the patch? 6. I wasn't aware I could easily change the top of a patch without problems. Would I still be going into $S, copying the working tree, editing, and diffing? The reason the patch works, as I understand it, is portage will try varying arguments to the -p flag in `patch`. Up to 4, I believe, so that means it'll try removing up to four directories from the paths in order to find matches. I can clean up the patch and attempt the rename if you'd like.
Daniel; 1 & 2. There's no prob here. No need for sorry. It's fine. As I said the naming is arbitrary. The new name is fine. Whether the old is kept or substituted with the new is optional so let's go for keeping the name 1.3.5-generate_menu-printf-fix.patch simply because it's less disruptive to update all and achieves the same original purpose. This is a grey area. Another dev might change the name but both 'work'. 3. Right that's clear now. 4. No prob, I shall add it here now. 5. Normally yes. 6. However, replacing the whole thing with a patch from an upstream commit replaces the whole thing anyway. This is what I've done. The most convenient way to add a patch. Just observe the final patch and you'll observe I've simply done it myself, nothing you can't or haven't done already. See ChangeLog entry in 537356.
