Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 12 Nov 2015 08:40:27 +0100
From:      John Marino <freebsd.contact@marino.st>
To:        Bryan Drewery <bdrewery@FreeBSD.org>, Alexey Dokuchaev <danfe@FreeBSD.org>
Cc:        ports-committers@freebsd.org, svn-ports-all@freebsd.org, svn-ports-head@freebsd.org
Subject:   Re: svn commit: r401299 - head/security/openssh-portable/files
Message-ID:  <5644426B.7000908@marino.st>
In-Reply-To: <56440462.6000803@FreeBSD.org>
References:  <201511112121.tABLLjO6051679@repo.freebsd.org> <20151112021225.GB43902@FreeBSD.org> <5643FC04.4020001@FreeBSD.org> <20151112030538.GA71430@FreeBSD.org> <56440462.6000803@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On 11/12/2015 4:15 AM, Bryan Drewery wrote:
> If you have to tell people to ignore a warning, the warning should come
> OUT or be changed.

To be fair, we'd prefer that the person heed the warning and regenerate
the patch.  However, some people rather spend 30 minutes complaining
that they shouldn't have to than 2 minutes regenerating the patches.  We
say "ignore it / grep it out" to folks that take the above stance
because it's not worth another discussion.

> Conditioning people to ignore warnings whenever they feel like it, or
> because the warnings are often false-positive, is not productive. It's
> why I spent so much time making check-plist correct last year in r351587.

Portlint is not gospel.  There are plenty of emitted warnings that on
stuff that is okay.


> The problem here is not "repo churn", it is creating busy work for
> people. It is unfortunate that portlint is growing stuff like this and
> the actually wrong advice of sorting Uses, 

Re: sorting: I'd argue that USES tools that clobber each other to
redefine tool names are wrong.  Especially if multiple versions of the
same tool are need for the port.  If we need fmake and gmake and/or
regular make then probably the port should be patched to use a single
tool (for example)


> since it is just creating
> work for people where work is not needed. There's probably < 3 people
> who care about these "consistency" issues. We should not push back on
> contributors because they missed an optional / or did not generate their
> patch with -p or started the comment with an 'A' (where upstream may
> even have it). It's counter-productive.

There is a distinction between "not needed" and "required"/"desired".
Regenerating patches is not required, but it is desired if they port is
being worked on substantially anyway.


> This check considers patches with header comments to be wrong. That's
> not right and I'm sure it was just overlooked. We should be doing the
> opposite though, encouraging comments in patches as to why they are
> there and their upstream status, rather than telling people to blindly
> blow away useful information.

This is spreading misinformation.  It checks the header to determine if
the patch is old or home-grown.  It doesn't have to every single missing
aspect.  If the "UTC" is missing, there's a very high probabibilty that
the patch is not in the current makepatch format.

Re: comments in patches.
You can get carried away.  You know how danfe is on consistency?  There
are people in pkgsrc that rip you every time you submit a patch w/o a
comment, even the change is obvious and self explanatory.  No thank you.

the takeaway:
"make makepatch" needs to be improved to do two things

1) conserve existing comments during regeneration
2) If an existing patch touches one or more files, the regenerated
version needs to cover the same files as the patch it replaces.


> I do think it is worth having -p generated diffs, but 'makepatch' did
> not do that until relatively recently. So this warning will appear to be
> false-positive to people who know they did use 'makepatch' in the past.

This is exaggerated.  Even if they were so confused, it would last
exactly one port when "svn diff" shows the difference.


> However, I don't agree with the warning since it's really asking people
> to do your work and lacks the larger vision of things like upstream
> status and whatever else I cannot think of (WE NEED MORE VISION IN
> PORTS). 

"your work"?
It's not "his" work, and w/o people, people revert good work of others.
 This is a fact, this is why the warning appeared in portlint because it
was happening A LOT.


> Looking at the original PR I see no evidence that this warning
> was added to catch actual bad patches, but only to encourage people to
> generate them in the new format. I keep beating this drum, let's not
> make people do busy work unless there's a really good reason. 

You make it sound like it doesn't take 20 seconds to 2 minutes to do it
on top of a port overhaul they are doing anyway.


> New PLIST format? Great, but make it worth doing, not just for the sake of it
> looking prettier, make it happen with sub-packages. There's 24k ports,
> we need things like provides/requires, not whitespace consistency
> distractions.


This is an entirely different topic, one that you misrepresented the
first time and news flash: There is no such thing as "subpackages" right
now.  Any work on that can *EASILY* adjust.  we're talking minutes to
improve on existing PROTOYPE code.  Let's not ambush this thread though.

John








Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?5644426B.7000908>