pkgsrc-Changes archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: CVS commit: pkgsrc/devel/scmgit-base



Hans Rosenfeld <rosenfeld%grumpf.hope-2000.org@localhost> writes:

> On Fri, Feb 17, 2012 at 09:54:17AM -0500, Greg Troxel wrote:
>> 
>> "Hans Rosenfeld" <hans%netbsd.org@localhost> writes:
>> 
>> > Module Name:       pkgsrc
>> > Committed By:      hans
>> > Date:              Fri Feb 17 13:54:02 UTC 2012
>> >
>> > Modified Files:
>> >    pkgsrc/devel/scmgit-base: Makefile
>> >
>> > Log Message:
>> > Fix build on SunOS.
>> 
>> Can you explain why this is needed (in this case, and in general, in the
>> commit messages)?
>
> Yes, of course I can. Obviously it fails to link without it.
>
> Normally this should be handled by the BUILDLINK_LDADD.iconv variable in
> converters/libiconv/buildlink3.mk, but that's been broken for years and
> even after many hours trying to debug this I have no idea why. So I'll
> just use the same fix that's been in other packages for ages.
>
>>   While I can more or less see that this change
>> shouldn't affect other than SunOS (presumably Solaris only these days),
>> I don't follow why this is needed, and if so why it isn't an upstream
>> bug that should be reported there and have a tracker URL (ok, this is
>> git, a mailinglist post) in a comment.
>
> It builds without problems outside of pkgsrc, so I don't really see why
> I should report it.

> Seems I mixed something up here, thats completely unrelated since we
> were talking about libintl and not libiconv. Sorry for any confusion
> caused.

So, it seems this is workaround for a bug in pkgsrc.  In that case, I'd
like to see a PR filed about the bug, and a comment referencing the PR
in every workaround, much like we ask that patch files have comments and
links to upstream bugtracker entries.  Putting in non-understood
workarounds without comments leads to accumulation of lore and new
people trying to understand pkgsrc reaching the wrong conclusion.

Also, I think the commit messages should indicate that a workaround was
added and reference the PR.  "Fix build on FooOS" as a commit message
does not lead to the reader understanding the change (since it doesn't
actually summarize the change, but rather describes one effect of the
change), and I don't how anyone could make a good decision as to whether
to go read the diff or not.

In this case, I wonder how the git build on NetBSD gets libintl
(probably it is just looking in base, since I think bl3 doesn't hide
that), and if the issue is that git should explicitly depend on
devel/gettext-lib.

Attachment: pgpaTGPekP4xn.pgp
Description: PGP signature



Home | Main Index | Thread Index | Old Index