Page MenuHomeFreeBSD

newvers.sh: fix git false positive -dirty tag
ClosedPublic

Authored by emaste on Jun 22 2018, 4:36 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Sep 24, 7:57 AM
Unknown Object (File)
Sun, Sep 14, 6:23 AM
Unknown Object (File)
Fri, Sep 12, 6:54 PM
Unknown Object (File)
Sep 10 2025, 10:56 PM
Unknown Object (File)
Sep 4 2025, 6:59 AM
Unknown Object (File)
Aug 31 2025, 9:37 AM
Unknown Object (File)
Aug 3 2025, 2:07 PM
Unknown Object (File)
Jul 11 2025, 7:06 AM
Subscribers

Details

Summary

Assuming that any output from git diff-index --name-only implies changes in the working tree results in false positives: files with metadata, but not content, changes are also listed.

Check that content differences exist before adding the -dirty tag to the git hash.

PR: 229230

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

emaste added a subscriber: jilles.
sys/conf/newvers.sh
93 ↗(On Diff #44311)

This will be $git_cmd not git.

Hrm, this only works if git diff is run at the top level dir; needs further investigation.

cd to the VCS topdir before invoking git diff

Can you use --shortstat instead of parsing sha's ?

In D15968#376629, @kib wrote:

Can you use --shortstat instead of parsing sha's ?

No, the user-facing commands have the false positive problem identified in the comment, and trying to use plain git diff has performance concerns.

sys/conf/newvers.sh
80 ↗(On Diff #44314)

Declare locals?

83 ↗(On Diff #44314)

"latter"

94 ↗(On Diff #44314)

IMO it would be a bit nicer to cache foo=$(realpath "$VCSTOP") (to ensure that it's an absolute path), and then git diff "${foo}/${file}". Then you don't need the subshell.

markj added inline comments.
sys/conf/newvers.sh
80 ↗(On Diff #44314)

There's also smode, dmode, ssha, dsha, status, file.

This revision is now accepted and ready to land.Nov 2 2018, 9:10 PM
emaste added inline comments.
sys/conf/newvers.sh
80 ↗(On Diff #44314)

The while loop has an implicit subshell and the loop variables themselves aren't visible outside so I think they're not needed.

This revision was automatically updated to reflect the committed changes.