On Sun, Aug 20 2017, Donovan Watteau <[email protected]> wrote:
> On Tue, 8 Aug 2017, Jeremie Courreges-Anglas wrote:
>> On Tue, Aug 01 2017, Donovan Watteau <[email protected]> wrote:
>> > Hi,
>> >
>> > $ cat pkg/DESCR
>> > dosfstools consists of the programs mkfs.fat, fsck.fat and fatlabel
>> > to create, check and label file systems of the FAT family.
>> >
>> > I've been using this for a few months on loongson and macppc (especially
>> > fsck.fat, which a bit more useful than base fsck_mdos(8)), without any
>> > particular problem.
>> >
>> > "make test" is OK as well.
>> 
>> Looks good
>> 
>> Here's an updated tarball with the following tweaks:
>> - detect xxd at "make test" time instead of configure time,
>
> Ah yeah, good idea, and simple patch, nice!
>
>> and move editors/vim,-main to TEST_DEPENDS
>
> I think you forgot to actually move it to TEST_DEPENDS (it's now
> missing), so I've added it back, in my updated tarball.

Pfff...

>> - don't use -Wl,--as-needed in LDFLAGS (is there a good reason?).
>
> mkfs.fat was linked against iconv without using it, so it was a quick
> workaround, but on second thought it's not really fixing something for
> us.  More of a small detail I should report to upstream, instead.

ok, thanks for the explanation.

>>   Instead, append our current LDFLAGS instead of clobbering them
>
> I don't mind this, but I see lots and *lots* of ports just doing
> LDFLAGS="-L${LOCALBASE}/lib".  So I'm not sure whether it's really
> an issue?

Not a *big* issue, however it's always better when a port respects
CC/CFLAGS/LDFLAGS... Here for example DEBUG=-g would not result in
LDFLAGS=-g being used at link time.  I have no interest in fixing all
affected ports, but I'd like new ports to respect this.

>> I'd probably move V above DISTNAME,
>
> Looks like most ports do this indeed (including an earlier port of mine),
> so ok.
>
>> and EXTRACT_SUFX below MASTER_SITES, but enough nitpicking for today. :)
>
> I see both idioms in the ports tree, and I think I prefer EXTRACT_SUFX
> near DISTNAME (because I see the suffix more as a property of the
> distfile, rather than a property of the mirror), but I don't mind,
> really :)

Fine with me.

> So here's an updated tarball, mostly adding editors/vim,-main back as
> a dependency.

Updated tarball attached with the LDFLAGS tweak mentioned above.
I think this is good for import. ok?

Attachment: dosfstools.tgz
Description: Binary data

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Reply via email to