Page MenuHomePhabricator

gn build: Use "git rev-parse --git-dir" to discover the path to the .git directory.
ClosedPublic

Authored by pcc on Mon, Jan 7, 5:51 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

pcc created this revision.Mon, Jan 7, 5:51 PM
llvm/utils/gn/build/write_vcsrevision.py
52 ↗(On Diff #180595)

For the record, on Python3, subprocess.check_output generates bytes and not str. It seems to be ok as an argument to os.path.isdir, but it requires an extra decode() after the strip()

pcc marked an inline comment as done.Tue, Jan 8, 3:57 PM
pcc added inline comments.
llvm/utils/gn/build/write_vcsrevision.py
52 ↗(On Diff #180595)

I tried the modified script with Python 3 and it appeared to work as is. However, I did notice a difference in behaviour between Python 2 and 3, which I've sent a fix for in D56459.

thakis accepted this revision.Wed, Jan 9, 6:05 AM
thakis added inline comments.
llvm/utils/gn/build/write_vcsrevision.py
35 ↗(On Diff #180595)

should this use exists() too to be consistent with line 38?

This revision is now accepted and ready to land.Wed, Jan 9, 6:05 AM
serge-sans-paille accepted this revision.Wed, Jan 9, 7:13 AM
serge-sans-paille added inline comments.
llvm/utils/gn/build/write_vcsrevision.py
52 ↗(On Diff #180595)

great, LGTM then.

pcc marked an inline comment as done.Thu, Jan 10, 1:57 PM
pcc added inline comments.
llvm/utils/gn/build/write_vcsrevision.py
35 ↗(On Diff #180595)

The reason why I switched to exists for .git is that .git can either be a file or a directory (because of worktrees). As far as I know .svn will always be a directory so it seems slightly more correct to use isdir here.

This revision was automatically updated to reflect the committed changes.