Page MenuHomeFreeBSD

add a new port: textproc/tttcmds
ClosedPublic

Authored by daichi on Jul 2 2018, 9:37 AM.

Details

Summary

add a new port: textproc/tttcmds

I have not been able to commit for a while, so I am currently in a reactivate state.
It is necessary to check with the active ports committer. My current mentor is gnn,
but he wishes to have other committers review the ports. Thank you.

Test Plan

% cd textproc/tttcmds
% make install clean
...snip...
% rehash
% which retusel
/usr/local/bin/retusel
% env LANG=C retusel -h
NAME
retu_select - print the specified columns in the specified order

SYNOPSIS
retu_select [-1] [-e] [-hvD] [--] N|N/M|N.k.s|N/M.k.s [N|N/M|
N.k.s|N/M.k.s ...] [file ...]

DESCRIPTION
Print the specified columns in the specified order.
...snip...
% make deinstall

> Deinstalling for tttcmds

> Deinstalling tttcmds-1.0.20180701

Updating database digests format: 100%
Checking integrity... done (0 conflicting)
Deinstallation has been requested for the following 1 packages (of 0 packages in the universe):

Installed packages to be REMOVED:
tttcmds-1.0.20180701

Number of packages to be removed: 1

The operation will free 2 MiB.
[1/1] Deinstalling tttcmds-1.0.20180701...
[1/1] Deleting files for tttcmds-1.0.20180701: 100%
%

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

daichi created this revision.Jul 2 2018, 9:37 AM
gnn requested changes to this revision.Jul 10 2018, 3:00 AM

Please find a ports person (I don''t commit enough ports) to review this as well please.

This revision now requires changes to proceed.Jul 10 2018, 3:00 AM
daichi edited the summary of this revision. (Show Details)Jul 10 2018, 3:10 AM
daichi edited reviewers, added: jbeich; removed: gnn.
eadler added a subscriber: eadler.Jul 10 2018, 3:41 AM
eadler added inline comments.
misc/tttcmds/Makefile
1 ↗(On Diff #44749)

we don't add this anymore.

10 ↗(On Diff #44749)

no need to repeat the command name here. I'm also not quite sure what a "development commands set" is

13 ↗(On Diff #44749)

no need for this: its implicit in being BSD2CLAUSE (I think - its been a while)

danfe added a subscriber: danfe.Jul 10 2018, 3:49 AM
danfe added inline comments.
misc/tttcmds/Makefile
7 ↗(On Diff #44749)

misc is a last resort category; is there really no better, more specific category for it?

10 ↗(On Diff #44749)

Bad COMMENT: it starts with the software name (which is specifically advised against in the PHB) and does not describe it very well actually. Something along Unix-style data processing commands and library would be better.

17 ↗(On Diff #44749)

This is the default value for GH_PROJECT and thus should be dropped.

19 ↗(On Diff #44749)

Why is it -jX unsafe? Can this be fixed? At the very least it should be explained in the commit log or accompanied by the explanatory comment.

misc/tttcmds/distinfo
1 ↗(On Diff #44749)

How did you generate the distinfo file, by hand? Because it's missing TIMESTAMP line and you won't be able to commit it (would be blocked by the hook).

misc/tttcmds/pkg-descr
6 ↗(On Diff #44749)

Typo in the word favorite. Also, I think operating systems are more common than operation ones.

8 ↗(On Diff #44749)

This is redundant: you already define LICENSE in the Makefile, don't repeat yourself.

misc/tttcmds/pkg-plist
112 ↗(On Diff #44749)

Oh, not even a README file? It's OK, but good software should come with documentation.

danfe added inline comments.Jul 10 2018, 3:51 AM
misc/tttcmds/Makefile
1 ↗(On Diff #44749)

We don't, but it's fine.

daichi updated this revision to Diff 45100.Jul 10 2018, 5:12 AM
daichi retitled this revision from add new port: misc/tttcmds to add a new port: misc/tttcmds.
daichi edited the summary of this revision. (Show Details)

I changed according to advice of eadler and danfe. Thank you.

daichi retitled this revision from add a new port: misc/tttcmds to add a new port: textproc/tttcmds.Jul 10 2018, 5:15 AM
daichi edited the summary of this revision. (Show Details)
daichi edited the test plan for this revision. (Show Details)
mat added a subscriber: mat.Jul 10 2018, 7:44 AM
mat added inline comments.
textproc/tttcmds/Makefile
16 ↗(On Diff #45100)

I do not understand that sentence, what does not correspond with what exactly?

daichi updated this revision to Diff 45110.EditedJul 10 2018, 1:16 PM

I changed according to lwhs' advice. Thank you.

(tttcmds is developed only for amd64. It can not be built on i386.)

danfe added inline comments.Jul 10 2018, 4:34 PM
textproc/tttcmds/Makefile
16 ↗(On Diff #45100)

I wouldn't be so hard here on @daichi: while the message is not perfect, it gives enough context for those interested in fixing it.

13 ↗(On Diff #45110)

It's good manners to include ONLY_FOR_ARCHS_REASON, but frankly, I find it hard to believe that a genetic data processing would be inherently amd64-only.

I'll try to build it on i386 and maybe send some patches.

textproc/tttcmds/pkg-plist
113 ↗(On Diff #45110)

For a single docfile, you might want to use PORTDOCS variable instead.

danfe added a comment.Jul 10 2018, 4:49 PM
...
util_millisecond.c:40:23: error: format specifies type 'long' but the argument has type 'time_t' (aka 'int') [-Werror,-Wformat]
        printf("%ld%03ld\n", tv.tv_sec, tv.tv_usec / 1000);
                ~~~          ^~~~~~~~~
                %d

It looks like pretty minor issue; most certainly it can be made arch-agnostic, and ONLY_FOR_ARCH lifted.

daichi updated this revision to Diff 45136.Jul 11 2018, 4:06 AM

I changed according to danfe's advice. Thank you.

daichi updated this revision to Diff 45205.Jul 12 2018, 12:31 PM

Changed to correspond to make - jX build

mat added inline comments.Jul 12 2018, 12:40 PM
textproc/tttcmds/Makefile
18 ↗(On Diff #45205)

You cannot use PORTDOCS without a DOCS option. Please add OPTIONS_DEFINE=DOCS.

daichi updated this revision to Diff 45206.Jul 12 2018, 12:54 PM

Changed Makefile according to mat's advice. Thank you.

  • GNU objcopy (e.g., via USE_BINUTILS) cannot be called after chmod when building as non-root, see error log
  • CFLAGS are hardcoded in vendor Makefile, so debugging via ASan or profiling optimized builds isn't possible
  • WITH_DEBUG builds are unconditionally stripped as vendor Makefile uses a different variable
  • -fstack-protector-strong and -Wno-unused-local-typedef aren't supported by base Clang 3.4.1 on FreeBSD 10.*, see error log
  • -Qunused-arguments, -Wmissing-variable-declarations and some -Wno-* are not supported by GCC, see error log and GCC warnings
  • -Werror is fragile on compiler upgrades and should probably be hidden behind MAINTAINER_MODE or something similar
textproc/tttcmds/Makefile
11 ↗(On Diff #45206)

Any reason you've dropped LICENSE_FILE? BSD2CLAUSE isn't available under /usr/ports/Templates/Licenses/, so when building in LICENSES_ASK=1 mode the dialog window would show a stub instead of license text.

textproc/tttcmds/pkg-plist
37 ↗(On Diff #45206)

devel/open-usp-tukubai installs the same file. Maybe define CONFLICTS_INSTALL in both ports.

57 ↗(On Diff #45206)

devel/open-usp-tukubai installs the same file. Maybe define CONFLICTS_INSTALL in both ports.

112 ↗(On Diff #45206)

USE_LDCONFIG in Makefile maybe required e.g., for LIB_DEPENDS to work. Even portlint -C complains about this.

ld(1) cannot find dynamic library due to missing *.so symlink.

$ echo 'int main() {}' >a.c
$ cc a.c -lttt -L/usr/local/lib -Wl,--verbose | fgrep ttt
attempt to open /usr/local/lib/libttt.so failed
attempt to open /usr/local/lib/libttt.a succeeded

ABI tracking:

$ make stage-qa
[...]
Warning: /usr/ports/textproc/tttcmds/work/stage/usr/local/lib/libttt.so.0 doesn't have a SONAME.
Warning: pkg(8) will not register it as being provided by the port.
Warning: If another port depend on it, pkg will not be able to know where it comes from.
Warning: It is directly in /usr/local/lib, it is probably used by other ports.
113 ↗(On Diff #45206)

%%PORTDOCS%% in pkg-plist acts like a conditional i.e., expands to either empty string (DOCS=on) or @comment (DOCS=off). PORTDOCS defined in Makefile automatically registers the files (as if they were in pkg-plist). So, either drop this line or use %%PORTDOCS%%%%DOCSDIR%%/README.md then drop PORTDOCS in Makefile.

$ make -V '${PLIST_SUB:MPORTDOCS*}' WITH=DOCS
PORTDOCS=""
$ make -V '${PLIST_SUB:MPORTDOCS*}' WITHOUT=DOCS
PORTDOCS="@comment "
$ fgrep README $(make -V TMPPLIST)
/usr/local/share/doc/tttcmds/README.md
daichi updated this revision to Diff 45230.Jul 13 2018, 5:23 AM

Thank you for your detailed advice, jbeich. It has been rewritten according to jbeich's advice.

However, the SONAME problem has not been solved. I need your help.

daichi updated this revision to Diff 45905.Jul 27 2018, 11:28 AM

Fixed SONAME problem.

I fixed all issues that were pointed out. I will commit unless there is a problem in particular.
Thank you.

Looks good but some D16095#344641 issues are not fixed:

  • GNU objcopy pulled via USE_BINUTILS or due to non-default PATH (e.g., /usr/local/bin before /usr/bin) still breaks build, see error log
  • -Qunused-arguments as used by bundled one-true-awk still breaks USE_GCC builds, see error log
daichi updated this revision to Diff 46714.Aug 15 2018, 4:08 PM

Fixed the problems pointed out by jbeich:

  • build error about objcopy and ongs_awk
  • build error for building by gcc
This revision was not accepted when it landed; it landed in state Needs Review.Sep 20 2018, 1:20 PM
This revision was automatically updated to reflect the committed changes.
mat added inline comments.Sep 21 2018, 8:15 AM
head/textproc/tttcmds/Makefile
24
post-install-DOCS-on:

Mat advice reflected. Thank you.