svn commit: r410612 - head/devel/libc++

[Bryan Drewery]

On 3/9/2016 12:00 AM, John Marino (NetBSD) wrote:
> On 2016-03-09 06:00, Bryan Drewery wrote:
>> This is a synth bug and should be reverted.
> 
> Right now make -V LIB_DEPENDS shows accurately under every platform
> where it did not before.  Who would wish to revert it?  Or in other
> words, who thinks the previous version of libc++ is "more correct" than
> the current version?

It was accurate before though and is now redundant. The way synth treats
the result is inaccurate. The real behavior here is in 'make
lib-depends', not 'make -V LIB_DEPENDS' which contains 0 logic. I had
intended to upstream Poudriere's dependency check here into a 'make
actual-lib-depends-list' or something but never did so.  It may even be
redundant with some of the new work from bapt in Mk/Scripts now.

> 
>> LIB_DEPENDS is already conditional on the specified library being
>> missing.  The tool should only add the dependency if the library is not
>> present because that is the behavior of LIB_DEPENDS (and *_DEPENDS) in
>> ports without tools, meaning it is the intended behavior.  This was
>> broken in portupgrade until r387621
> 
> I was 90% sure LIB_DEPENDS was designed to do what it was doing, but
> frankly I think it's technically a terrible design decision.  Now that
> you have two build tools that were negatively affected by it, that
> opinion has merit.
> 
> In other words, if anything, I'd like to formally re-evaluate that
> decision that have LIB_DEPENDS not being trustworthy as being ok with
> the goal of getting concurrence that it must be trustworthy.  e.g.
> LIB_DEPENDS most propagate to the built package and if it doesn't it's
> wrong.

The issue is with all _DEPENDS. RUN_DEPENDS= foo:origin is only applied
when foo doesn't already exist in PATH. Same for FETCH/PATCH/BUILD
depends, but it's only relevant here for RUN_DEPENDS and LIB_DEPENDS due
to registering in the package.

I don't like conditionalizing it here because it creates a dependency of
the built package on the system that built it. That sounds absurd but
consider that LIB_DEPENDS is simply METADATA. The METADATA should
generally be available to the package regardless of what was on the
system that build it. Longterm it may make sense to have a package
generated and use this LIB_DEPENDS as an optional dependency if it's not
on the installed system.

> 
> 1) There cannot be many ports left in the tree that were doing what
> libc++ was doing.  e.g. < 5 ?

I think it will be far larger than that. We don't document nor commonly
add exists() checks around RUN_DEPENDS and LIB_DEPENDS.  From the last
time I looked at this the usage of it was fairly low.

> 
> 2) it's easy to set LIB_DEPENDS conditionally
> 
> 3) the current way is a POLA violation (IMO) and causes problems with
> package validity checks.  It is an unwanted side effect and it's easily
> fixed.  It requires people to understand technicallhy what package is
> doing?

This is why there is hesitation to throw away good tools and write new
ones.  The existing behavior must be respected and otherwise fixed in
all tools.  One does not simply write a replacement tool without
understanding all of the behavior.  Poudriere took years to get right
and it still is wrong in regards to handling DEPENDS_ARGS.

Portmaster is still wrong in this case but the impact is that unneeded
things get installed, rather than rebuilt. No one seemed to notice I am
guessing because the actual problem of the dependency
already-being-satisfied and existing already is rare.

> 
> Can we do the right thing here?
> 

Regardless of this specific commit, synth should be fixed to not assume
RUN_DEPENDS and LIB_DEPENDS are unconditional, they are metadata and the
logic of lib-depends/run-depends should be used instead.

-- 
Regards,
Bryan Drewery

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freebsd.org/pipermail/svn-ports-head/attachments/20160309/66f47a85/attachment.sig>