Page MenuHomeFreeBSD

Unbreak lang/picoc
Needs ReviewPublic

Authored by krion on Mar 23 2017, 11:46 AM.
Tags
None
Referenced Files
Unknown Object (File)
Jan 1 2024, 6:43 AM
Unknown Object (File)
Nov 19 2023, 9:34 AM
Unknown Object (File)
Nov 12 2023, 6:29 PM
Unknown Object (File)
Oct 15 2023, 8:20 AM
Unknown Object (File)
Oct 8 2023, 5:24 PM
Unknown Object (File)
Sep 20 2023, 10:27 AM
Unknown Object (File)
Jul 7 2023, 10:49 PM
Unknown Object (File)
Mar 23 2017, 8:10 PM
Subscribers
None

Details

Reviewers
mat
Summary

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=218023

Unbreak lang/picoc by fixing MASTER_SITES
Pass maintainership to submitter
Fix WWW URL

Diff Detail

Repository
rP FreeBSD ports repository
Lint
No Lint Coverage
Unit
No Test Coverage
Build Status
Buildable 8250
Build 8493: arc lint + arc unit

Event Timeline

Seeing all the changes in the patches, it is not the same as the previous 2.1 version, so PORTVERSION should be updated, maybe 2.1g<commit-date>, doing that, the commit message should look more like:

Update to a later commit and follow upstream change
Pass maintainership to submitter

Don't mention the WWW change in the commit message, it is implied by the fact that upstream changed.

I re-did the patches with "make makepatch" to make portlint happy, I can point it out in commit log.

Did you approve or left for changes this revision, I couldn't identify it from your comment :-)

You can't have just run "make patch" then "make makepatch" there are too many things changing to be the result of only that.

I made them manually and did makepatch, what could be wrong with it? Anyway, I don't care and will bring back old files/* versions.

What I mean is that if you need to change the patches manually, then it is not the same upstream version, and then, PORTVERSION must change.

Sorry, did I understand it right, if I make patches conform with portlint(1) and run "make makepatch" I've to bump PORTVERSION ?? Where's documented, what policy applies to it and what the reason behind it?

No, running make makepatch to get portlint to stop complaining (which is something you should not do) does not require you to bump PORTREVISION. On the other hand, like I was saying, this is not what is happening here, the patches are changing way to much for it to be the he result of just make makepatch.

When you run make makepatch, what may change is:

  1. the patch header, with the file name and the date
  2. the hunk headers where the line numbers move
  3. the context of the hunks, that is, the lines that begin with a space

That's it. Here, you have hunks going away, and hunks being changed.

Mat, and now look at this picture, I totally understand what you mean, I totally understand this point. What we're doing here is - discussing the need to change the patches to shut up portlint warnings or not. I totally agree - a) it could lead to unneeded problems if you're not careful enough b) there's not much sense to change it because we change just hunk positions and c) we can interfere with spaces and tabs and break things.

Now let's calculate the moves I've done before sending it to you:
a) reviewed every patch in files/ to understand what it does and why, looking into src
b) applied manually changes to files
c) run make makepatch and generated new patches to shut up portlint
d) run make test and poudriere build to make sure it builds properly
e) filed arc revision and sent it
f) you've reviewed and dived into comments
g) we've started with philosophy around changing patches

Should I calculate the most valuable asset we had and which we've lost for these reviews and changes - time. Instead of - "no-go, because of a) b) c) - redo" or "go ahead, approved". Mat, the project degraded a lot because of all of that talkie-talkie stuff, I'm noticing it by reading our mailing lists, because "everyone and my dog" would like to comment on everything around what moves and what we see. Sorry for that, maybe, rude points, but that what I see in the project currently and it makes me worry :)

Ok, let's backtrack a moment.

What is happening here is upstream changed, and so has the distfile, it went from picoc-2.1.tar.bz2 that was ~70kB to zsaleeba-picoc-2.1-4555e84_GH0.tar.gz that is about 610kB.

So, the content of the distfile changed, a lot. We're in the "What is the proper procedure for updating the checksum for a port's distfile when the file changes without a version change" case here.

As the distfile changed a lot, I'm nitpicking about the patches because it feels like the code changed.

There are two cases:

  1. The differences are outside of the code that is being built, say, the new distfile brings in some doc that is not installed, or something, then it can be committed as is, but you have to mention in the commit message that you did check the two distfiles and summarize the differences.
  2. There are differences in the code, or the files that are installed, so it is really a different version, and then, it is no longer version 2.1 but some other version, and in that case, the PORTVERSION must be changed to reflect that, as it is based on a commit, what we usually do is call it 2.1g20160817 or whatever the commit date is.

I don't know that you checked that the distfile content is the same or not, you have not spoken about it, this is, again, why I try to make sure the patches changes are normal, which is why I have not said "you need to change this" or "this is ok", I'm still waiting to see if something has to change or not.


As for the reviews we do after the fact, yes, the project has changed, it has evolved. Getting what you do reviewed by your peers is a good thing, if someone raises a point, it means that something is wrong/not great/not clear, and that it could maybe be done in a better way. I do not feel at all that it has degraded, some things may take a bit more time to do, but in the end, it means they are done in a better way and we all learn things.

My intention was to bump PORTEPOCH for that, as we did all the time after rerolling distfile.

In terms of review, pkg, poudriere you're right - project evolved significantly, but in terms of intercommunications and relations between people inside of project it degraded dramatically

I'm sure you meant PORTREVISION. But that is only the case if the distfile is still the same version. Here, it is not the case, the distfile has 2.1 in its name.

But in that case, any change you mean to do to the port when you commit it must be present here, in this review, because you are still being mentored, and you cannot commit anything that has not been approved.
Also, the commit message must state why you bumped PORTREVISION, or changed PORTVERSION because it was no longer accurate, but first and foremost, you must make the effort to summarize the differences.

Until some of this are done, here, in this review, it cannot go forward..

Yes I mean PORTREVISION, not PORTEPOCH and not PORTVERSION