Created attachment 145874 [details] revised bump_revision.pl I am attaching a rewritten bump_revision.pl script with these major changes: - possible to give multiple ports on the command line, for instance, ilmbase and OpenEXR, so that all portrevisions of ports that depend on either will be bumped - a new method (-l option) to limit bumping revisions of ports with direct dependencies (slow, uses make -C cat/port -V LIB_DEPENDS -V RUN_DEPENDS on each of the candidates), - better hunt for INDEX files (looks at INDEX-10 or thereabouts) - updated for SVN - robustness fixes and internal processing fixes, fewer loops where we can map.
First of all I apologize for this taking two weeks for me getting back to. (It's funny, I've been the only one making improvement to this for a couple of years, essentially whenever I used it, so I ended up inheriting it, and now there is wider interest it seems. ;-) I would have preferred a few independent patches, which are easier to review, but let me give it a try in one piece: - Can you please add a note on this adding -T to the options for Perl to the commit message? - In the initial comment, I don't think we have to mention that this is Perl script. For the user that should not matter. - Why does it "help with bumping" as opposed to just "bump"? - I'm not convinced /usr/ports is what most people using this script will be using for their work; how about saying "such as"? - I am not necessarily in agreement setting $ENV{'PATH'} as such. Can you please revert this and we can discuss separately from the other changes? - Shouldn't we just default to one of -l and -g, and I guess -g since that's currently the case? - Can you please add a comment above "$| = 1;" for those not too familiar with Perl? - The print "$TMPDIR/.svn exists, do you want to use the -n option? Press Ctrl-C to abort.\n"; sleep 5; part feels interesting. I don't think we should make the result of running this script dependent on whether someone got distracted or was looking at her screen. - # If specified as portname, check all indexes for existence or duplicates There is only one INDEX in use, so this is a bit confusing. I assume you mean "check the entire INDEX" or so? - Instead of using ".bump_rev_tmpdir.", how about using the name of the script? - Sometimes it reads ".... Aborting", sometimes "..., aborting". I suggest to always go for the former. - Should direct_dependence by called direct_dependency? I assume you see why I would have preferred three, four smaller patches which I believe would have been less effort overall for both of us? Anyway, there is a bit more, but we can tackle that later. If you consider the input above, I'm fine with these changes. And I do trust your Perl skills which clearly are far superior to mine :-), so did not review the more intricate bits in detail.
Created attachment 205635 [details] updates version for modern perl, deletes variable named ENV to avoid shell abuse This revised version adds a "delete $ENV{'ENV'};" so as to avoid a /bin/sh from the Perl system function from pulling in dangerous goods, and to satisfy stricter requirements of Perl's. Gerald, if you want to hand over maintainership to me, feel free to do so, and commit this new revised revision (file date 2018-09-09) with a changed MAINTAINER= line and let me know. Note the file name is changed, you need to "mv" it into place before committing. Further considerations behind the scenes by e-mail. We may want to give fellow contributors a short help text discussing advantages of the .sh vs. the .pl version.
Comment on attachment 205635 [details] updates version for modern perl, deletes variable named ENV to avoid shell abuse Uh, I haven't addressed Gerald's concerns from the original yet. Withdrawing approval request.
Created attachment 205636 [details] overhauled version heeding Gerald's comments This version addresses Gerald's comments, except the $ENV{PATH} override. We need to set a defined PATH so we don't inherit shit from the environment, and it's technically a requirement for running with strict taint checks (perl -T, see man perlrun and man perlsec). Further improvement might be through Getopt::Long and long options and better documentation.
This is a nice revamp, thank you! There is one recommendation I have, and perhaps an idea: Last year I added a safety net to the bump-revision.sh script, that works as follows: # See what the port thinks its PORTREVISION is and save that. pre=$(make -C "$1" -V PORTREVISION) : # See what the port now has for PORTREVISION. post=$(make -C "$1" -V PORTREVISION) if [ "$post" -le "$pre" ]; then printc "ERROR: $1 PORTREVISION went backwards from $pre to $post !" "red" fi That is, use the actual Ports machinery to determine the actual PORTREVISION and ensure that does not go backwards. This can happen when there is no PORTREVISION set in a Makefile, we add one, but it actually is set somewhere else via .include. See `svn log -c477561` for more details. Now the idea: bump-revision.sh is very simplistic compared to this script. Perhaps leave the logic and intelligence to this script, and leverage the simple bump-revision.sh script for the actual bumping?
A commit references this bug: Author: gerald Date: Fri Jul 12 12:20:34 UTC 2019 New revision: 506455 URL: https://svnweb.freebsd.org/changeset/ports/506455 Log: Transfer maintainership of this script to mandree@ based on conversations we've been having after I was planning to EOL it last year and a nice revamp of this script he has prepared. Thank you for taking this over, and good luck, Matthias! PR: 192706 Changes: head/Tools/scripts/bump_revision.pl
A commit references this bug: Author: mandree Date: Fri Jul 12 16:07:45 UTC 2019 New revision: 506466 URL: https://svnweb.freebsd.org/changeset/ports/506466 Log: Update bump_revision.pl. Key features: - updated for SVN (the old version expected CVS) - shallow mode to only bump direct dependencies (option -l) PR: 192706 Submitted by: mandree@ (new maintainer) Changes: head/Tools/scripts/bump_revision.pl
Thanks, Gerald.