Re: [NEW] sysutils/dosfstools

On Sun, Aug 20 2017, Donovan Watteau <tsoomi@gmail.com> wrote:
> On Tue, 8 Aug 2017, Jeremie Courreges-Anglas wrote:
>> On Tue, Aug 01 2017, Donovan Watteau <tsoomi@gmail.com> 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?