Skip to content
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

Add doctor command to write commit-graphs #20007

Merged
merged 6 commits into from
Jun 25, 2022
Merged

Add doctor command to write commit-graphs #20007

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b130407
Add doctor command to write commit-graphs
zeripath Jun 17, 2022
1dd2570
Merge branch 'main' into doctor-commitgraph
zeripath Jun 17, 2022
56cd7d2
placate lint
zeripath Jun 17, 2022
6b0a5b6
Merge branch 'main' into doctor-commitgraph
techknowlogick Jun 24, 2022
6408bbe
Merge branch 'main' into doctor-commitgraph
zeripath Jun 25, 2022
9b5175d
Merge branch 'main' into doctor-commitgraph
zeripath Jun 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions modules/doctor/mergebase.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ func iteratePRs(ctx context.Context, repo *repo_model.Repository, each func(*rep
}

func checkPRMergeBase(ctx context.Context, logger log.Logger, autofix bool) error {
git.InitOnceWithSync(ctx)
zeripath marked this conversation as resolved.
Show resolved Hide resolved
numRepos := 0
numPRs := 0
numPRsUpdated := 0
Expand Down
74 changes: 74 additions & 0 deletions modules/doctor/misc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,73 @@ func checkDaemonExport(ctx context.Context, logger log.Logger, autofix bool) err
return nil
}

func checkCommitGraph(ctx context.Context, logger log.Logger, autofix bool) error {
git.InitOnceWithSync(ctx)

numRepos := 0
numNeedUpdate := 0
numWritten := 0
if err := iterateRepositories(ctx, func(repo *repo_model.Repository) error {
numRepos++

commitGraphExists := func() (bool, error) {
// Check commit-graph exists
commitGraphFile := path.Join(repo.RepoPath(), `objects/info/commit-graph`)
isExist, err := util.IsExist(commitGraphFile)
if err != nil {
logger.Error("Unable to check if %s exists. Error: %v", commitGraphFile, err)
return false, err
}

if !isExist {
commitGraphsDir := path.Join(repo.RepoPath(), `objects/info/commit-graphs`)
isExist, err = util.IsExist(commitGraphsDir)
Copy link
Member

@delvh delvh Jun 17, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't there a case missing that this directory is not empty?
Or what is the file format inside this directory?
Shouldn't this be checked as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think its worth checking for the format of the directory and/or file at this point.

If you are really interested in how commit graphs are created you are welcome to peruse:

https://github.com/git/git/blob/master/Documentation/technical/commit-graph.txt

and

https://github.com/git/git/blob/master/Documentation/technical/commit-graph-format.txt

Here is the salient points for the contents of commit-graphs

A commit-graph chain uses multiple files, and we use a fixed naming convention
to organize these files. Each commit-graph file has a name
`$OBJDIR/info/commit-graphs/graph-{hash}.graph` where `{hash}` is the hex-
valued hash stored in the footer of that file (which is a hash of the file's
contents before that hash). For a chain of commit-graph files, a plain-text
file at `$OBJDIR/info/commit-graphs/commit-graph-chain` contains the
hashes for the files in order from "lowest" to "highest".

We could assert that the chain file exists instead of the directory I guess.

if err != nil {
logger.Error("Unable to check if %s exists. Error: %v", commitGraphsDir, err)
return false, err
}
}
return isExist, nil
}

isExist, err := commitGraphExists()
if err != nil {
return err
}
if !isExist {
numNeedUpdate++
if autofix {
if err := git.WriteCommitGraph(ctx, repo.RepoPath()); err != nil {
logger.Error("Unable to write commit-graph in %s. Error: %v", repo.FullName(), err)
return err
}
isExist, err := commitGraphExists()
if err != nil {
return err
}
if isExist {
numWritten++
logger.Info("Commit-graph written: %s", repo.FullName())
zeripath marked this conversation as resolved.
Show resolved Hide resolved
} else {
logger.Warn("No commit-graph written: %s", repo.FullName())
zeripath marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
return nil
}); err != nil {
logger.Critical("Unable to checkCommitGraph: %v", err)
return err
}

if autofix {
logger.Info("Wrote commit-graph files for %d of %d repositories.", numWritten, numRepos)
} else {
logger.Info("Checked %d repositories, %d without commit-graphs.", numRepos, numNeedUpdate)
}

return nil
}

func init() {
Register(&Check{
Title: "Check if SCRIPT_TYPE is available",
Expand Down Expand Up @@ -225,4 +292,11 @@ func init() {
Run: checkDaemonExport,
Priority: 8,
})
Register(&Check{
Title: "Check commit-graphs",
Name: "check-commit-graphs",
IsDefault: false,
Run: checkCommitGraph,
Priority: 9,
})
}