Created attachment 191731 [details] sysutils/dupd: update to version 1.6 Update dupd to version 1.6. dupd no longer depends on libbloom so that dependency is removed. Also, upstream source now supports FreeBSD directly so no patches needed.
Thanks. Still a few issues unfortunately: CFLAGS=-m64 -DDIRENT_HAS_TYPE -m64 seems unnecessary, is this meant to only work on amd64? It seemed to work OK on 32-bit, but I didn't test it much. USAGE_ARCH=-O elf64-x86-64 -B i386 $(OBJCP) -I binary $(USAGE_ARCH) man/dupd $(BUILD)/usage.o As you see in patch-Makefile, I replaced this with: ld -r -b binary -o $(BUILD)/usage.o man/dupd Which avoided having to hardcode bfdarch. As per #226997 this might need switching to ld.bfd. I'm also wary of the Makefile's GITHASH stuff - at best it does nothing, possibly spewing a "Command not found", at worst it stuffs some git-mirrored port checkout hash in there.
What is 'bfd' in this context? In general I'd recommend keeping patched lines to a minimum (with zero being optimal) if only to simplify future version upgrades (so there is no need to tweak patches each time). I'm happy to include any configurability or FreeBSD needs in the upstream Makefile (as long as it remains cross-platform). To the specific points: The GITHASH definition is harmless even if git not present. The value is only ever used in development builds, so because this port build is meant for release builds, it it irrelevant. It is safe to patch it out as before, but see above on minimizing patches. The -m64 flag is unnecessary. However, I don't know if it'll work correctly as a 32bit build. I have not tried that in many releases/years. Is that important for FreeBSD's needs? As to the objcopy/ld line, I'll look into a way to make this more flexible in a way that meets the needs on all platforms. That'd be for next release, so I guess you can retain that patch for this time. Thanks for the initial port!
> What is 'bfd' in this context? GNU ld, basically. It uses GNU's Binary File Descriptor library, so I guess ld.bdf makes more sense than ld.gnu. *shrug*. > The GITHASH definition is harmless even if git not present. The value > is only ever used in development builds, so because this port build is > meant for release builds, it it irrelevant. It is safe to patch it out > as before, but see above on minimizing patches. OK. I mainly patched it out because I don't like seeing spurious "command not found" errors in my build logs - it's one more thing to remember to ignore when testing a port and one more thing for users to get worried over ("why is this giving me fatal git errors during build?!"). If the Makefile set it conditionally it'd be easy to disable just by passing in a definition to override it. > The -m64 flag is unnecessary. However, I don't know if it'll work > correctly as a 32bit build. I have not tried that in many > releases/years. Is that important for FreeBSD's needs? If it only supports certain architectures it should be set ONLY_FOR_ARCHS=, but I'd rather only do that if it's specifically known- broken. Luckily there is a test suite :) Either way, it's blatting ports-provided CFLAGS, which is to be avoided: https://www.freebsd.org/doc/en/books/porters-handbook/book.html#dads-cflags > As to the objcopy/ld line, I'll look into a way to make this more > flexible in a way that meets the needs on all platforms. That'd be for > next release, so I guess you can retain that patch for this time. How about: system("man dupd"); ;)
Found more fun regarding the ld stuff: https://news.ycombinator.com/item?id=10816322 Specifically, readelf -lW shows dupd has an executable stack: GNU_STACK ... RWE 0x8 Fixing this involves even more hoop jumping. This seems a disproportionate amount of effort for a feature that is literally less useful than just typing "man dupd". Instead of chasing it, I'm going to replace that function with a simple call to man(1), followed by the banner so it shows up after the user leaves the man page. Will have an updated patch up shortly, currently running tests.
Boo: ---------- test.72 -------------------- OK scan(files8) OK report 4,9d3 < < < < < < 11,16d4 < 10 < 11 < 12 < 13 < 14 < 3 18,26c6,7 < 6 < 7 < 8 < 9 < 33 total bytes used by duplicates: < 36 total bytes used by duplicates: < 60 total bytes used by duplicates: < 76 total bytes used by duplicates: < Total used: 205 bytes (0 KiB, 0 MiB, 0 GiB) --- > 22 total bytes used by duplicates: > Total used: 22 bytes (0 KiB, 0 MiB, 0 GiB) FAIL report: verifying against output.72 error: test.72 exit code: 1 gmake[1]: *** [Makefile:119: test] Error 1
i386: ---------- test.55 -------------------- OK scan(files3): large files OK generate report 10,11c10,11 < 0 total bytes used by duplicates: < 112210190 total bytes used by duplicates: --- > 4407177486 total bytes used by duplicates: > 8589934592 total bytes used by duplicates: FAIL generate report: verifying against output.55 error: test.55 exit code: 1 gmake[1]: *** [Makefile:119: test] Error 1
Created attachment 192352 [details] sysutils/dupd: update to version 1.6, again Due to the test failure on large files I've marked it NOT_FOR_ARCHS there. The test failure on amd64 is a result of running the test as root, which can bypass 000 permissions - running it as a normal user completes fine. I've hooked up make test to the test suite so it can be ran from the port. patch-Makefile: remove GITHASH, set LIB and INC to search PREFIX, remove CFLAGS override. patch-srch_main.c: Replace show_usage() embedded manpage with a call to `man system` followed by the banner. portlint is happy, poudriere testport is happy on 10.4 and 11.1 amd64.
On the embedded manpage, for this case it could just as well just print "see manpage" or similar, although invoking man should be fine as well. The embedded manpage is really meant for users who don't have the manpage installed (which I guess is most users who try dupd, compiling directly from github source). But for the specific case of this port, since the port delivers the manpage already, it's not particularly interesting to embed it.
Created attachment 192363 [details] sysutils/dupd: update to version 1.6, again again Add missing perl5 in USES.
Luca? This has sat about for a few months now, be nice to get it in before 1.7 turns up :)
I'm pretty close to a 1.7 release, so I just filed 229773 to preliminarily track that. IMO it would still be nice to complete the 1.6 port just so there is continuity of package versions. But if that's too much work I understand.
(In reply to Thomas Hurst from comment #10) Hi Thomas, really sorry, I don't know how, but my reply get lost, probably in my head... I do remember, that when I tried to build it, I had some compile errors. I'm rebuilding it and I'll be back to you.
So, my last build succeeded, so I'll commit it asap. sorry again for the delay...
A commit references this bug: Author: pizzamig Date: Mon Jul 16 12:56:57 UTC 2018 New revision: 474735 URL: https://svnweb.freebsd.org/changeset/ports/474735 Log: sysutils/dupd: Update to version 1.6 PR: 226841 Submitted by: jyri@virkki.com Reviewed by: tom@hur.st (maintainer) Changes: head/sysutils/dupd/Makefile head/sysutils/dupd/distinfo head/sysutils/dupd/files/patch-Makefile head/sysutils/dupd/files/patch-src_utils.h
Committed, thanks!
I'm reopening the PR because it doesn't build. An error in my build pipeline built dupd 1.4. In the meanwhile, I've marked the port as BROKEN, waiting for a fix. The error with dupd 1.6 is: /wrkdirs/usr/ports/sysutils/dupd/work/dupd-1.6/build/main.o: In function `show_usage': src/main.c:(.text+0x10da): undefined reference to `_binary_man_dupd_start' src/main.c:(.text+0x10e0): undefined reference to `_binary_man_dupd_end' I gave a look and those _binary_man_dupd symbols are missing for whatever reason.
I guess the Makefile patch went out of sync somewhere, now sure how it got there. But, see discussion above on the embedded manpage. Not really worth dealing with for this particular use case (FreeBSD port) because port delivers man page anyway. As Thomas suggested earlier, easiest just to replace it with a comment for 1.6. Something like this: diff --git a/src/main.c b/src/main.c index 2b8863b..56cc142 100644 --- a/src/main.c +++ b/src/main.c @@ -143,17 +143,10 @@ static void show_usage() { show_banner(); -#ifndef __APPLE__ - char * p = &_binary_man_dupd_start; - while (p != &_binary_man_dupd_end) { - putchar(*p++); - } -#else - printf("Usage documentation not available on Darwin!\n"); + printf("For manpage, run 'man dupd'\n"); printf("\n"); printf("Alternatively, refer to the document here:\n"); printf("https://github.com/jvirkki/dupd\n"); -#endif }
Created attachment 195186 [details] Patch adding missing patch Ugh, yeah, my patch missed files/patch-src_main.c, should have reviewed more closely. Fix attached.
A commit references this bug: Author: pizzamig Date: Wed Jul 18 08:12:41 UTC 2018 New revision: 474840 URL: https://svnweb.freebsd.org/changeset/ports/474840 Log: sysutils/dupd: Fix the broken build An issue in my build pipeline allowed me to not notice the broken build. This commit add a patch to fix the build, so the port is available again PR: 226841 Submitted by: tom@hur.st (maintaner) Changes: head/sysutils/dupd/Makefile head/sysutils/dupd/files/patch-src_main.c
Finally correctly pushed to the repo. Thanks for your patience and support. Next time I'll follow my PR better, promise.