Skip to content

cmd/go/internal/vcs: also check file mode when identifying VCS root #56296

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 3 commits into from
Closed

cmd/go/internal/vcs: also check file mode when identifying VCS root #56296

wants to merge 3 commits into from

Conversation

ZekeLu
Copy link
Contributor

@ZekeLu ZekeLu commented Oct 18, 2022 •
edited
Loading

Currently, FromDir identifies a VCS checkout directory just by checking
whether it contains a specified file. This is not enough. For example,
although there is a ".git" file (a plain file, not a directory) in a
git submodule directory, this directory is not a git repository root.

This change takes the file mode into account. As of now, the filename
and file mode for the supported VCS tools are:

  • Mercurial: .hg directory
  • Git: .git directory
  • Bazaar: .bzr directory
  • Subversion: .svn directory
  • Fossil: .fslckout plain file
  • Fossil: FOSSIL plain file

This CL effectively reverts CL 30948 for #10322.

Fixes #53640.

@gopherbot
Copy link
Contributor

This PR (HEAD: 272fc3b) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/443597 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Bryan Mills:

Patch Set 1:

(2 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/443597.
After addressing review feedback, remember to publish your drafts!

@ZekeLu ZekeLu changed the title cmd/go/internal/vcs: avoid treating git submodule directory as repository root cmd/go/internal/vcs: also check file mode when identifying VCS root Nov 2, 2022
@gopherbot
Copy link
Contributor

This PR (HEAD: c05c951) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/443597 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Zeke Lu:

Patch Set 2:

(3 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/443597.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Bryan Mills:

Patch Set 4: Run-TryBot+1

(2 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/443597.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gopher Robot:

Patch Set 4:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/443597.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gopher Robot:

Patch Set 4:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/443597.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gopher Robot:

Patch Set 4: TryBot-Result-1

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/443597.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Bryan Mills:

Patch Set 4:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/443597.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Zeke Lu:

Patch Set 4:

(2 comments)

Please don’t reply on this GitHub thread. Visit golang.org/cl/443597.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

This PR (HEAD: 11aaf22) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/443597 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Zeke Lu:

Patch Set 5:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/443597.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Bryan Mills:

Patch Set 5: Run-TryBot+1 Code-Review+2

Please don’t reply on this GitHub thread. Visit golang.org/cl/443597.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gopher Robot:

Patch Set 5:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/443597.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gopher Robot:

Patch Set 5:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/443597.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gopher Robot:

Patch Set 5: TryBot-Result-1

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/443597.
After addressing review feedback, remember to publish your drafts!

ZekeLu added 3 commits November 3, 2022 00:53
@ZekeLu
cmd/go/internal/vcs: avoid treating git submodule directory as reposi…
81922ad 
…tory root

Before the change, FromDir treats a directory contains a .git entry
as a git repository root, no matter whether the entry is a directory
or a normal file. When git submodule is used, the directory of the
submodule contains a .git file (it's a normal file, not a directory).
In this case, the directory of this submodule should not be treated
as the repository root.
@ZekeLu
cmd/go/internal/vcs: also check file mode when identifying VCS root
973cdc4 
Currently, FromDir identifies a VCS checkout directory just by checking
whether it contains a specicil file. This is not enough. For example,
although there is a ".git" file (a plain file, not a directory) in a
git submodule directory, this directory is not a git repository root.

This change takes the file mode into account. As of now, the filename
and file mode for the supported VCS tools are:

- Mercurial:	.hg		directory
- Git:		.git		directory
- Bazaar:	.bzr		directory
- Subversion:	.svn		directory
- Fossil:	.fslckout	plain file
- Fossil:	_FOSSIL_	plain file

This CL effectively reverts CL 30948 for golang#10322. Dmitri left a comment
(golang#10322 (comment))
there. But it's unfortunately ignored.
@ZekeLu
fix TestScript/version_buildvcs_fossil
7a2d6ff
@gopherbot
Copy link
Contributor

This PR (HEAD: 7a2d6ff) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/443597 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Contributor

Message from Zeke Lu:

Patch Set 5:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/443597.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Bryan Mills:

Patch Set 6: Run-TryBot+1 Auto-Submit+1 Code-Review+2

Please don’t reply on this GitHub thread. Visit golang.org/cl/443597.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gopher Robot:

Patch Set 6:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/443597.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gopher Robot:

Patch Set 6:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/443597.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gopher Robot:

Patch Set 6: TryBot-Result-1

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/443597.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Bryan Mills:

Patch Set 6:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/443597.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Bryan Mills:

Patch Set 6: Run-TryBot+1

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/443597.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gopher Robot:

Patch Set 6:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/443597.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Contributor

Message from Gopher Robot:

Patch Set 6: TryBot-Result+1

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/443597.
After addressing review feedback, remember to publish your drafts!

gopherbot pushed a commit that referenced this pull request Nov 3, 2022
@ZekeLu @gopherbot
cmd/go/internal/vcs: also check file mode when identifying VCS root
bb39656 
Currently, FromDir identifies a VCS checkout directory just by checking
whether it contains a specified file. This is not enough. For example,
although there is a ".git" file (a plain file, not a directory) in a
git submodule directory, this directory is not a git repository root.

This change takes the file mode into account. As of now, the filename
and file mode for the supported VCS tools are:

- Mercurial:    .hg             directory
- Git:          .git            directory
- Bazaar:       .bzr            directory
- Subversion:   .svn            directory
- Fossil:       .fslckout       plain file
- Fossil:       _FOSSIL_        plain file

This CL effectively reverts CL 30948 for #10322.

Fixes #53640.

Change-Id: Iea316c7e983232903bddb7e7f6dbaa55e8498685
GitHub-Last-Rev: 7a2d6ff
GitHub-Pull-Request: #56296
Reviewed-on: https://go-review.googlesource.com/c/go/+/443597
Auto-Submit: Bryan Mills <[email protected]>
Run-TryBot: Bryan Mills <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Than McIntosh <[email protected]>
Reviewed-by: Bryan Mills <[email protected]>
@gopherbot
Copy link
Contributor

This PR is being closed because golang.org/cl/443597 has been merged.

@gopherbot gopherbot closed this Nov 3, 2022
romaindoumenc pushed a commit to TroutSoftware/go that referenced this pull request Nov 3, 2022
@ZekeLu @romaindoumenc
cmd/go/internal/vcs: also check file mode when identifying VCS root
cd4f24d 
Currently, FromDir identifies a VCS checkout directory just by checking
whether it contains a specified file. This is not enough. For example,
although there is a ".git" file (a plain file, not a directory) in a
git submodule directory, this directory is not a git repository root.

This change takes the file mode into account. As of now, the filename
and file mode for the supported VCS tools are:

- Mercurial:    .hg             directory
- Git:          .git            directory
- Bazaar:       .bzr            directory
- Subversion:   .svn            directory
- Fossil:       .fslckout       plain file
- Fossil:       _FOSSIL_        plain file

This CL effectively reverts CL 30948 for golang#10322.

Fixes golang#53640.

Change-Id: Iea316c7e983232903bddb7e7f6dbaa55e8498685
GitHub-Last-Rev: 7a2d6ff
GitHub-Pull-Request: golang#56296
Reviewed-on: https://go-review.googlesource.com/c/go/+/443597
Auto-Submit: Bryan Mills <[email protected]>
Run-TryBot: Bryan Mills <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Than McIntosh <[email protected]>
Reviewed-by: Bryan Mills <[email protected]>
@ZekeLu ZekeLu deleted the git-submodule branch November 4, 2022 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

cmd/go: go build fails to read vcs info from isolated git submodule
2 participants
@ZekeLu @gopherbot