mfc-candidates: Minor improvements
ClosedPublic
Actions

Authored by jrm on Mon, Sep 22, 6:28 PM.

Details

Reviewers

emaste
andrew
jrtc27

Group Reviewers

releng

Commits

rGe57d72a7f35c: mfc-candidates: Improve branch detection and repository handling

Summary

Use git to detect the latest stable branch, rather than hardcoding it, but only run the git detection when the working branch is not already a stable or releng branch.
Handle the case where the script is run outside a src or ports repository.
Fix a pattern to match .git instead of *git`.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

jrm created this revision.Mon, Sep 22, 6:28 PM

Herald added a subscriber: imp. · View Herald TranscriptMon, Sep 22, 6:28 PM

jrm requested review of this revision.Mon, Sep 22, 6:28 PM

Harbormaster completed remote builds in B67241: Diff 162583.Mon, Sep 22, 6:28 PM

Won't that cause problems for stable/15 when stable/16 is branched?

Add a comment before the git command that extracts the latest stable branch.

Harbormaster completed remote builds in B67253: Diff 162641.Tue, Sep 23, 12:48 PM

@andrew, it will be the same situation as now, when we have to choose whether to list MFC candidates for stable/14 or stable/15 by default. If I understood our IRC conversation correctly, you requested mfc-candidates be updated to tell you your stable/15 MFC candidates by default. This change does that and will automatically do it for stable/16 as soon as stable/17 is tagged.

When there are multiple active stable branches, users can set pwd to check MFC candidates for other branches. For example, with this change, if you want to see MFC candidates for stable/14, check out stable/14 and run the script. When stable/17 is tagged and you want to see MFC candidations for stable/15, check out stable/15 before running the script.

emaste added inline comments.Tue, Sep 23, 1:55 PM

tools/tools/git/mfc-candidates.lua
144	Setting `to_branch` here is the fallback if we don't pick up a branch from the working tree; it would be good to avoid execing git to fetch the latest branch if we're just going to throw it away in many cases. Better would be to set it to nil here, and add this as below the `string.match(cur_branch` cases below

Incorporate @emaste's suggestion.

Also, when the script was run from outside of a git repository, the output would be

fatal: not a git repository (or any parent up to mount point /usr)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
/usr/libexec/flua: /usr/src/tools/tools/git/mfc-candidates.lua:134: bad argument #1 to 'gsub' (string expected, got nil)
stack traceback:
        [C]: in function 'string.gsub'
        /usr/src/tools/tools/git/mfc-candidates.lua:134: in local 'main'
        /usr/src/tools/tools/git/mfc-candidates.lua:217: in main chunk
        [C]: in ?

With the added error checking, it's simply

pwd is not under a ports or src repository.

Harbormaster completed remote builds in B67257: Diff 162663.Tue, Sep 23, 4:26 PM

jrm marked an inline comment as done.Tue, Sep 23, 4:28 PM

emaste added inline comments.Tue, Sep 23, 4:35 PM

tools/tools/git/mfc-candidates.lua
152	sorry for not being explicit earlier -- `to_branch` is optionally set by the two earlier cases, so we want to allow them to set it if appropriate, or choose the default if neither picked up a default

jrm added inline comments.Tue, Sep 23, 5:51 PM

tools/tools/git/mfc-candidates.lua
152	Sorry if I'm being dense, but isn't that what's happening? Before: Set `to_branch` to `stable/14` If the checked out branch is `stable/x` or `releng/x`, overwrite `to_branch` with it. After: If the checked out branch is `stable/x` or `releng/x`, use it. Otherwise, use the latest stable branch. The overall logic is the same. The only difference is that the default branch is not hard-coded, and now the new git command is only run when the checked out branch is neither `stable/x` nor `releng/x`. Did you mean something different by "choose the default"?