Skip to content

Commit 82171be

Browse files
authored
feat(internal/librarian): only generate libraries with changed APIs (#2618)
The new -generate-unchanged flag can be used to generate all (unblocked) libraries. Libraries which are blocked by configuration are still not generated when -generate-unchanged is specified. Specifying -library ignores this check (and blocking) and will always generate the specified library. Fixes #818
1 parent 45652c0 commit 82171be

File tree

10 files changed

+537
-92
lines changed

10 files changed

+537
-92
lines changed

cmd/librarian/doc.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,9 @@ Flags:
104104
-build
105105
If true, Librarian will build each generated library by invoking the
106106
language-specific container.
107+
-generate-unchanged
108+
If true, librarian generates libraries even if none of their associated APIs
109+
have changed. This does not override generation being blocked by configuration.
107110
-host-mount string
108111
For use when librarian is running in a container. A mapping of a
109112
directory from the host to the container, in the format

internal/config/config.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,12 +123,18 @@ type Config struct {
123123
// expected.
124124
CommandName string
125125

126-
// Commit determines whether to creat a commit for the release but not create
126+
// Commit determines whether to create a commit for the release but not create
127127
// a pull request.
128128
//
129129
// This flag is ignored if Push is set to true.
130130
Commit bool
131131

132+
// GenerateUnchanged determines whether to generate libraries where none of
133+
// the associated APIs have changed since the commit at which they were last
134+
// generated. Note that this does not override any configuration indicating
135+
// that the library should not be automatically generated.
136+
GenerateUnchanged bool
137+
132138
// GitHubAPIEndpoint is the GitHub API endpoint to use for all GitHub API
133139
// operations.
134140
//

internal/gitrepo/gitrepo.go

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ type Repository interface {
5555
CleanUntracked(paths []string) error
5656
pushRefSpec(refSpec string) error
5757
Checkout(commitHash string) error
58+
GetHashForPath(commitHash, path string) (string, error)
5859
}
5960

6061
const RootPath = "."
@@ -419,20 +420,20 @@ func commitMatchesPath(path string, commit *object.Commit, parentCommit *object.
419420
if path == RootPath {
420421
return true, nil
421422
}
422-
currentPathHash, err := getHashForPathOrEmpty(commit, path)
423+
currentPathHash, err := getHashForPath(commit, path)
423424
if err != nil {
424425
return false, err
425426
}
426-
parentPathHash, err := getHashForPathOrEmpty(parentCommit, path)
427+
parentPathHash, err := getHashForPath(parentCommit, path)
427428
if err != nil {
428429
return false, err
429430
}
430431
return currentPathHash != parentPathHash, nil
431432
}
432433

433-
// getHashForPathOrEmpty returns the hash for a path at a given commit, or an
434+
// getHashForPath returns the hash for a path at a given commit, or an
434435
// empty string if the path (file or directory) did not exist.
435-
func getHashForPathOrEmpty(commit *object.Commit, path string) (string, error) {
436+
func getHashForPath(commit *object.Commit, path string) (string, error) {
436437
tree, err := commit.Tree()
437438
if err != nil {
438439
return "", err
@@ -663,3 +664,19 @@ func (r *LocalRepository) Checkout(commitSha string) error {
663664
Hash: plumbing.NewHash(commitSha),
664665
})
665666
}
667+
668+
// GetHashForPath returns a tree hash for the specified path,
669+
// at the given commit in this repository. If the path does not exist
670+
// at the commit, an empty string is returned rather than an error,
671+
// as the purpose of this function is to allow callers to determine changes
672+
// in the tree. (A path going from missing to anything else, or vice versa,
673+
// indicates a change. A path being missing at two different commits is not a change.)
674+
func (r *LocalRepository) GetHashForPath(commitHash, path string) (string, error) {
675+
// This public function just delegates to the internal function that uses a Commit
676+
// object instead of the hash.
677+
commit, err := r.repo.CommitObject(plumbing.NewHash(commitHash))
678+
if err != nil {
679+
return "", err
680+
}
681+
return getHashForPath(commit, path)
682+
}

internal/gitrepo/gitrepo_test.go

Lines changed: 68 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -904,27 +904,34 @@ func TestGetDir(t *testing.T) {
904904
}
905905
}
906906

907-
func TestGetHashForPathOrEmpty(t *testing.T) {
907+
// TestGetHashForPath tests the internal getHashForPath, but
908+
// via the public GetHashForPath function which accepts a commit hash
909+
// instead of a Commit object, to avoid duplicate testing.
910+
func TestGetHashForPath(t *testing.T) {
908911
t.Parallel()
909912

910-
setupInitialRepo := func(t *testing.T) (*git.Repository, *object.Commit) {
913+
setupInitialRepo := func(t *testing.T) (LocalRepository, *object.Commit) {
911914
t.Helper()
912-
repo, _ := initTestRepo(t)
915+
repo, dir := initTestRepo(t)
913916
commit := createAndCommit(t, repo, "initial.txt", []byte("initial content"), "initial commit")
914-
return repo, commit
917+
localRepository := LocalRepository{
918+
Dir: dir,
919+
repo: repo,
920+
}
921+
return localRepository, commit
915922
}
916923

917924
for _, test := range []struct {
918925
name string
919-
setup func(t *testing.T) (commit *object.Commit, path string)
926+
setup func(t *testing.T) (repo LocalRepository, commit *object.Commit, path string)
920927
wantHash func(commit *object.Commit, path string) string
921928
wantErr bool
922929
}{
923930
{
924931
name: "existing file",
925-
setup: func(t *testing.T) (*object.Commit, string) {
926-
_, commit := setupInitialRepo(t)
927-
return commit, "initial.txt"
932+
setup: func(t *testing.T) (LocalRepository, *object.Commit, string) {
933+
localRepository, commit := setupInitialRepo(t)
934+
return localRepository, commit, "initial.txt"
928935
},
929936
wantHash: func(commit *object.Commit, path string) string {
930937
tree, err := commit.Tree()
@@ -940,8 +947,9 @@ func TestGetHashForPathOrEmpty(t *testing.T) {
940947
},
941948
{
942949
name: "existing directory",
943-
setup: func(t *testing.T) (*object.Commit, string) {
944-
repo, _ := setupInitialRepo(t)
950+
setup: func(t *testing.T) (LocalRepository, *object.Commit, string) {
951+
localRepository, _ := setupInitialRepo(t)
952+
repo := localRepository.repo
945953
// Create a directory and a file inside it to ensure the directory gets a hash
946954
_ = createAndCommit(t, repo, "my_dir/file_in_dir.txt", []byte("content of file in dir"), "add dir and file")
947955
head, err := repo.Head()
@@ -952,7 +960,7 @@ func TestGetHashForPathOrEmpty(t *testing.T) {
952960
if err != nil {
953961
t.Fatalf("repo.CommitObject failed: %v", err)
954962
}
955-
return commit, "my_dir"
963+
return localRepository, commit, "my_dir"
956964
},
957965
wantHash: func(commit *object.Commit, path string) string {
958966
tree, err := commit.Tree()
@@ -968,28 +976,29 @@ func TestGetHashForPathOrEmpty(t *testing.T) {
968976
},
969977
{
970978
name: "non-existent file",
971-
setup: func(t *testing.T) (*object.Commit, string) {
972-
_, commit := setupInitialRepo(t)
973-
return commit, "non_existent_file.txt"
979+
setup: func(t *testing.T) (LocalRepository, *object.Commit, string) {
980+
localRepository, commit := setupInitialRepo(t)
981+
return localRepository, commit, "non_existent_file.txt"
974982
},
975983
wantHash: func(commit *object.Commit, path string) string {
976984
return ""
977985
},
978986
},
979987
{
980988
name: "non-existent directory",
981-
setup: func(t *testing.T) (*object.Commit, string) {
982-
_, commit := setupInitialRepo(t)
983-
return commit, "non_existent_dir"
989+
setup: func(t *testing.T) (LocalRepository, *object.Commit, string) {
990+
localRepository, commit := setupInitialRepo(t)
991+
return localRepository, commit, "non_existent_dir"
984992
},
985993
wantHash: func(commit *object.Commit, path string) string {
986994
return ""
987995
},
988996
},
989997
{
990998
name: "file in subdirectory",
991-
setup: func(t *testing.T) (*object.Commit, string) {
992-
repo, _ := setupInitialRepo(t)
999+
setup: func(t *testing.T) (LocalRepository, *object.Commit, string) {
1000+
localRepository, _ := setupInitialRepo(t)
1001+
repo := localRepository.repo
9931002
_ = createAndCommit(t, repo, "another_dir/sub_dir/nested_file.txt", []byte("nested content"), "add nested file")
9941003
head, err := repo.Head()
9951004
if err != nil {
@@ -999,7 +1008,7 @@ func TestGetHashForPathOrEmpty(t *testing.T) {
9991008
if err != nil {
10001009
t.Fatalf("repo.CommitObject failed: %v", err)
10011010
}
1002-
return commit, "another_dir/sub_dir/nested_file.txt"
1011+
return localRepository, commit, "another_dir/sub_dir/nested_file.txt"
10031012
},
10041013
wantHash: func(commit *object.Commit, path string) string {
10051014
tree, err := commit.Tree()
@@ -1017,17 +1026,52 @@ func TestGetHashForPathOrEmpty(t *testing.T) {
10171026
t.Run(test.name, func(t *testing.T) {
10181027
t.Parallel()
10191028

1020-
commit, path := test.setup(t)
1029+
localRepository, commit, path := test.setup(t)
10211030

1022-
got, err := getHashForPathOrEmpty(commit, path)
1031+
got, err := localRepository.GetHashForPath(commit.Hash.String(), path)
10231032
if (err != nil) != test.wantErr {
1024-
t.Errorf("getHashForPathOrEmpty() error = %v, wantErr %v", err, test.wantErr)
1033+
t.Errorf("getHashForPath() error = %v, wantErr %v", err, test.wantErr)
10251034
return
10261035
}
10271036

10281037
wantHash := test.wantHash(commit, path)
10291038
if diff := cmp.Diff(wantHash, got); diff != "" {
1030-
t.Errorf("getHashForPathOrEmpty() mismatch (-want +got):\n%s", diff)
1039+
t.Errorf("getHashForPath() mismatch (-want +got):\n%s", diff)
1040+
}
1041+
})
1042+
}
1043+
}
1044+
1045+
// TestGetHashForPathBadCommitHash tests the one path not
1046+
// otherwise tested in TestGetHashForPath, where we can't
1047+
// get the commit for the hash.
1048+
func TestGetHashForPathBadCommitHash(t *testing.T) {
1049+
repo, dir := initTestRepo(t)
1050+
localRepository := LocalRepository{
1051+
Dir: dir,
1052+
repo: repo,
1053+
}
1054+
for _, test := range []struct {
1055+
name string
1056+
commitHash string
1057+
}{
1058+
{
1059+
name: "empty hash",
1060+
commitHash: "",
1061+
},
1062+
{
1063+
name: "invalid hash",
1064+
commitHash: "bad-hash",
1065+
},
1066+
{
1067+
name: "hash not in repo",
1068+
commitHash: "d93e160f57f0a6eccd6e230dd40f465988bede63",
1069+
},
1070+
} {
1071+
t.Run(test.name, func(t *testing.T) {
1072+
_, err := localRepository.GetHashForPath(test.commitHash, "path/to/file")
1073+
if err == nil {
1074+
t.Error("GetHashForPath() err = nil, should fail when an invalid or absent hash is provided")
10311075
}
10321076
})
10331077
}

internal/librarian/flags.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,12 @@ func addFlagCommit(fs *flag.FlagSet, cfg *config.Config) {
4545
a pull request. This flag is ignored if push is set to true.`)
4646
}
4747

48+
func addFlagGenerateUnchanged(fs *flag.FlagSet, cfg *config.Config) {
49+
fs.BoolVar(&cfg.GenerateUnchanged, "generate-unchanged", false,
50+
`If true, librarian generates libraries even if none of their associated APIs
51+
have changed. This does not override generation being blocked by configuration.`)
52+
}
53+
4854
func addFlagGitHubAPIEndpoint(fs *flag.FlagSet, cfg *config.Config) {
4955
fs.StringVar(&cfg.GitHubAPIEndpoint, "github-api-endpoint", "",
5056
`The GitHub API endpoint to use for all GitHub API operations.

0 commit comments

Comments
 (0)