Page MenuHomeFreeBSD

Convert ports tools (those in the public repository) to git.
ClosedPublic

Authored by rene on Mar 27 2021, 9:26 PM.

Details

Reviewers
emaste
lwhsu
uqs
rene
Group Reviewers
portmgr
Summary
  • Do not mention svn in search_lib_depends_and_bump.sh
  • MOVEDlint: convert to git (just remove svn compatibility)
  • convert rmport to git As discussed via email, the script will use an (optional) branching model of an existing tree instead of relying on ad-hoc sparse or partial checkouts. Do not provide got as an option. As naddy@ reports on git@, it is not an interchangeale alternative to git in this script. Simplify the workflow for a user a bit, use native git diff instead of svn diff >file ; cdiff < file Apply some shellcheck fixes and modernizations. Prefer --show-toplevel over --git-dir Use git diff --irreversible-delete instead of --diff-filter=d as the former mentions deleted files
  • Mk/bsd.java.mk: generalize a comment
  • Remove tools that are redundant with git (addport, mfh, psvn) Discussed with portmgr. Remove addport and mfh from README too
  • Convert tindex to git, only verified with shellcheck. Fix a shellcheck error at line 150
  • Tools/scripts/chkversion.pl: prune SVN

Diff Detail

Repository
rP FreeBSD ports repository
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 38170
Build 35059: arc lint + arc unit

Event Timeline

rene requested review of this revision.Mar 27 2021, 9:26 PM

Rebase

  • Do not mention svn in search_lib_depends_and_bump.sh
  • MOVEDlint: convert to git (just remove svn compatibility)
  • convert rmport to git
  • Mk/bsd.java.mk: generalize a comment
  • Remove tools that are redundant with git.
  • Remove addport and mfh from README too
  • Convert tindex to git, only verified with shellcheck.
  • Tools/scripts/chkversion.pl: prune SVN
uqs requested changes to this revision.Mar 29 2021, 8:53 AM
uqs added inline comments.
Tools/scripts/MOVEDlint.awk
40

when using git worktrees, .git is a file, not a dir. Change this to -r maybe?

Tools/scripts/rmport
44

out of curiosity: what does +0d actually accomplish?

48

being close to going live, let's point this to production already?

79–80

why fork another shell here? I think the previous was fine

115

the tr(1) munging makes this less readable, imho. The good thing about the ISO date format is that we can use regular string sorting and don't need to resort to converting them to numbers for almost another ~8000 years :)

154–155

broken double grep :)

345

having it immediately push w/o any sort of checking of what is being pushed seems a bit dangerous. Can we leave this to the user at first?

383–396

use git diff --exit-code here instead?

Tools/scripts/tindex
47

I think going with %ce is more future proof, we already have "committers" w/o @FreeBSD.org in src. we might relax that in the future ...

This revision now requires changes to proceed.Mar 29 2021, 8:53 AM

(would have been nice to put the shellcheck changes into another review, it would have made this one lighter to review.)

Tools/scripts/rmport
62–64

cdiff was renamed to ydiff upstream.

339–347

I know you're the only one using rmport, but I don't think this should end with a git push. It should probably only be doing this:

  • git commit --file=${gitlog}
  • git pull --rebase

And then, you can git amend your commit and push when you are ready.

But then, the workflow in here seems very complicated, maybe I got things wrong.

Tools/scripts/README
11

Ah, I missed this before committing rP569466. At least the conflict will be trivial to resolve :)

Tools/scripts/rmport
79–80

[ is a shell bulitin so I think they're equivalent, but I also think the previous one is clear.

rene marked 9 inline comments as done.Mar 29 2021, 6:01 PM
rene added inline comments.
Tools/scripts/rmport
44

Nothing :) I removed it.

48

Yep.

62–64

Ah, but cdiff / ydiff is irrelevant for git, because git diff has a (colorized) pager built-in, unlike svn diff

79–80

They are indeed equivalent, but according to POSIX the -a form is deprecated, see https://www.shellcheck.net/wiki/SC2166 and http://pubs.opengroup.org/onlinepubs/9699919799/utilities/test.html

115

Yes, but the \> operator is actually undefined in POSIX shell, see https://www.shellcheck.net/wiki/SC2039
And \> looks a bit like a redirect operator to me.

154–155

Yeah, a no-op grep I guess. Removed.

339–347

Hmm, the pull --rebase will not work here because it creates a temporary branch (as discussed on the git@ mailing list: https://lists.freebsd.org/pipermail/freebsd-git/2020-November/000525.html

345

Hm yes, I was following the SVN version, which did a commit by itself. It worked fine with the test repository though.

Tools/scripts/tindex
47

Yes, it is only used in the "Committers on the hook:" line in the "index failed" mail anyway.

rene marked 7 inline comments as done.
  • Do not mention svn in search_lib_depends_and_bump.sh
  • MOVEDlint: convert to git (just remove svn compatibility)
  • convert rmport to git
  • Mk/bsd.java.mk: generalize a comment
  • Remove tools that are redundant with git.
  • Remove addport and mfh from README too
  • Convert tindex to git, only verified with shellcheck.
  • Tools/scripts/chkversion.pl: prune SVN
  • Address issues from uqs
Tools/scripts/rmport
79–80

Huh, learned something new today :)

uqs requested changes to this revision.Mar 29 2021, 6:59 PM
uqs added inline comments.
Tools/scripts/rmport
115

then let's ignore shellcheck, as it is obviously dumb.

test(1) clearly states:

s1 = s2       True if the strings s1 and s2 are identical.

s1 != s2      True if the strings s1 and s2 are not identical.

s1 < s2       True if string s1 comes before s2 based on the binary value
              of their characters.

s1 > s2       True if string s1 comes after s2 based on the binary value
              of their characters.

the old version was more readable, escaping of > notwithstanding

This revision now requires changes to proceed.Mar 29 2021, 6:59 PM
  • Make rmport a bit more readable
rene marked an inline comment as done.Mar 29 2021, 8:59 PM
rene marked 2 inline comments as done.
  • Use git-diff --exit-code
rene marked an inline comment as done.Mar 29 2021, 9:03 PM
This revision is now accepted and ready to land.Apr 6 2021, 11:23 AM