Skip to content

Commit 8372878

Browse files
authored
refactor(internal): mv Publish to librarian/rust and use directly for librarian publish (#3485)
Moves Publish to librarian/rust and use directly for both sidekick rust-publish and librarian publish. This change: - moves `Publish` to `librarian/rust` and changes input from sidekick release to librarian release config. - rename publish_crates.go to publish.go - librarian publish to call rust.Publish directly, this avoids behavior drifts between sidekick and librarian commands. Fix #3376
1 parent 11000ee commit 8372878

File tree

6 files changed

+119
-198
lines changed

6 files changed

+119
-198
lines changed

internal/librarian/publish.go

Lines changed: 1 addition & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,7 @@ import (
1818
"context"
1919
"fmt"
2020

21-
"github.com/googleapis/librarian/internal/command"
2221
"github.com/googleapis/librarian/internal/config"
23-
"github.com/googleapis/librarian/internal/git"
2422
"github.com/googleapis/librarian/internal/librarian/rust"
2523
"github.com/googleapis/librarian/internal/yaml"
2624
"github.com/urfave/cli/v3"
@@ -54,42 +52,12 @@ func publishCommand() *cli.Command {
5452
}
5553

5654
func publish(ctx context.Context, cfg *config.Config, dryRun bool, skipSemverChecks bool) error {
57-
if err := verifyRequiredTools(ctx, cfg.Language, cfg.Release); err != nil {
58-
return err
59-
}
60-
gitExe := "git"
61-
if cfg.Release != nil {
62-
gitExe = command.GetExecutablePath(cfg.Release.Preinstalled, "git")
63-
}
64-
if err := git.AssertGitStatusClean(ctx, gitExe); err != nil {
65-
return err
66-
}
67-
lastTag, err := git.GetLastTag(ctx, gitExe, cfg.Release.Remote, cfg.Release.Branch)
68-
if err != nil {
69-
return err
70-
}
71-
files, err := git.FilesChangedSince(ctx, lastTag, gitExe, cfg.Release.IgnoredChanges)
72-
if err != nil {
73-
return err
74-
}
7555
switch cfg.Language {
7656
case languageFake:
7757
return fakePublish()
7858
case languageRust:
79-
return rust.PublishCrates(ctx, cfg.Release, dryRun, skipSemverChecks, lastTag, files)
59+
return rust.Publish(ctx, cfg.Release, dryRun, skipSemverChecks)
8060
default:
8161
return fmt.Errorf("publish not implemented for %q", cfg.Language)
8262
}
8363
}
84-
85-
// verifyRequiredTools verifies all the necessary tools are installed.
86-
func verifyRequiredTools(ctx context.Context, language string, cfg *config.Release) error {
87-
switch language {
88-
case languageFake:
89-
return nil
90-
case languageRust:
91-
return rust.PreFlight(ctx, cfg.Preinstalled, cfg.Remote, cfg.Tools["cargo"])
92-
default:
93-
return fmt.Errorf("unknown language: %s", language)
94-
}
95-
}
Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,29 @@ import (
3030
"github.com/googleapis/librarian/internal/git"
3131
)
3232

33-
// PublishCrates publishes the crates that have changed.
34-
func PublishCrates(ctx context.Context, config *config.Release, dryRun bool, skipSemverChecks bool, lastTag string, files []string) error {
33+
// Publish finds all the crates that should be published, (optionally) runs
34+
// `cargo semver-checks` and (optionally) publishes them.
35+
func Publish(ctx context.Context, config *config.Release, dryRun bool, skipSemverChecks bool) error {
36+
if err := PreFlight(ctx, config.Preinstalled, config.Remote, config.Tools["cargo"]); err != nil {
37+
return err
38+
}
39+
gitPath := command.GetExecutablePath(config.Preinstalled, "git")
40+
lastTag, err := git.GetLastTag(ctx, gitPath, config.Remote, config.Branch)
41+
if err != nil {
42+
return err
43+
}
44+
if err := git.MatchesBranchPoint(ctx, gitPath, config.Remote, config.Branch); err != nil {
45+
return err
46+
}
47+
files, err := git.FilesChangedSince(ctx, lastTag, gitPath, config.IgnoredChanges)
48+
if err != nil {
49+
return err
50+
}
51+
return publishCrates(ctx, config, dryRun, skipSemverChecks, lastTag, files)
52+
}
53+
54+
// publishCrates publishes the crates that have changed.
55+
func publishCrates(ctx context.Context, config *config.Release, dryRun bool, skipSemverChecks bool, lastTag string, files []string) error {
3556
manifests := map[string]string{}
3657
for _, manifest := range FindCargoManifests(files) {
3758
names, err := PublishedCrate(manifest)

internal/librarian/rust/publish_crates_test.go renamed to internal/librarian/rust/publish_test.go

Lines changed: 93 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ func TestPublishCratesSuccess(t *testing.T) {
5050
}
5151
lastTag := "release-2001-02-03"
5252

53-
if err := PublishCrates(t.Context(), cfg, true, false, lastTag, files); err != nil {
53+
if err := publishCrates(t.Context(), cfg, true, false, lastTag, files); err != nil {
5454
t.Fatal(err)
5555
}
5656
}
@@ -85,7 +85,7 @@ func TestPublishCratesWithNewCrate(t *testing.T) {
8585
path.Join("src", "pubsub", "src", "lib.rs"),
8686
}
8787
lastTag := "release-with-new-crate"
88-
if err := PublishCrates(t.Context(), cfg, true, false, lastTag, files); err != nil {
88+
if err := publishCrates(t.Context(), cfg, true, false, lastTag, files); err != nil {
8989
t.Fatal(err)
9090
}
9191
}
@@ -119,7 +119,7 @@ func TestPublishCratesWithRootsPem(t *testing.T) {
119119
path.Join("src", "storage", "src", "lib.rs"),
120120
}
121121
lastTag := "release-with-roots-pem"
122-
if err := PublishCrates(t.Context(), cfg, true, false, lastTag, files); err != nil {
122+
if err := publishCrates(t.Context(), cfg, true, false, lastTag, files); err != nil {
123123
t.Fatal(err)
124124
}
125125
}
@@ -158,7 +158,7 @@ func TestPublishCratesWithBadManifest(t *testing.T) {
158158
path.Join("src", "storage", "src", "lib.rs"),
159159
}
160160
lastTag := "release-2001-02-03"
161-
if err := PublishCrates(t.Context(), cfg, true, false, lastTag, files); err == nil {
161+
if err := publishCrates(t.Context(), cfg, true, false, lastTag, files); err == nil {
162162
t.Errorf("expected an error with a bad manifest file")
163163
}
164164
}
@@ -180,7 +180,7 @@ func TestPublishCratesGetPlanError(t *testing.T) {
180180
path.Join("src", "storage", "src", "lib.rs"),
181181
}
182182
lastTag := "release-2001-02-03"
183-
if err := PublishCrates(t.Context(), cfg, true, false, lastTag, files); err == nil {
183+
if err := publishCrates(t.Context(), cfg, true, false, lastTag, files); err == nil {
184184
t.Fatalf("expected an error during plan generation")
185185
}
186186
}
@@ -209,7 +209,7 @@ func TestPublishCratesPlanMismatchError(t *testing.T) {
209209
path.Join("src", "storage", "src", "lib.rs"),
210210
}
211211
lastTag := "release-2001-02-03"
212-
if err := PublishCrates(t.Context(), cfg, true, false, lastTag, files); err == nil {
212+
if err := publishCrates(t.Context(), cfg, true, false, lastTag, files); err == nil {
213213
t.Fatalf("expected an error during plan comparison")
214214
}
215215
}
@@ -254,11 +254,96 @@ fi
254254
lastTag := "release-2001-02-03"
255255

256256
// This should fail because semver-checks fails.
257-
if err := PublishCrates(t.Context(), cfg, true, false, lastTag, files); err == nil {
257+
if err := publishCrates(t.Context(), cfg, true, false, lastTag, files); err == nil {
258258
t.Fatal("expected an error from semver-checks")
259259
}
260260
// Skipping the checks should succeed.
261-
if err := PublishCrates(t.Context(), cfg, true, true, lastTag, files); err != nil {
261+
if err := publishCrates(t.Context(), cfg, true, true, lastTag, files); err != nil {
262262
t.Fatal(err)
263263
}
264264
}
265+
266+
func TestPublishSuccess(t *testing.T) {
267+
testhelper.RequireCommand(t, "git")
268+
testhelper.RequireCommand(t, "/bin/echo")
269+
cfg := &config.Release{
270+
Remote: "origin",
271+
Branch: "main",
272+
Preinstalled: map[string]string{
273+
"git": "git",
274+
"cargo": "/bin/echo",
275+
},
276+
Tools: map[string][]config.Tool{
277+
"cargo": {
278+
{Name: "cargo-semver-checks", Version: "1.2.3"},
279+
{Name: "cargo-workspaces", Version: "3.4.5"},
280+
},
281+
},
282+
}
283+
remoteDir := testhelper.SetupRepoWithChange(t, "release-2001-02-03")
284+
testhelper.CloneRepository(t, remoteDir)
285+
286+
if err := Publish(t.Context(), cfg, true, false); err != nil {
287+
t.Fatal(err)
288+
}
289+
}
290+
291+
func TestPublishWithLocalChangesError(t *testing.T) {
292+
testhelper.RequireCommand(t, "git")
293+
testhelper.RequireCommand(t, "/bin/echo")
294+
config := &config.Release{
295+
Remote: "origin",
296+
Branch: "main",
297+
Preinstalled: map[string]string{
298+
"git": "git",
299+
"cargo": "/bin/echo",
300+
},
301+
Tools: map[string][]config.Tool{
302+
"cargo": {
303+
{Name: "cargo-semver-checks", Version: "1.2.3"},
304+
{Name: "cargo-workspaces", Version: "3.4.5"},
305+
},
306+
},
307+
}
308+
remoteDir := testhelper.SetupRepoWithChange(t, "release-with-local-changes-error")
309+
testhelper.CloneRepository(t, remoteDir)
310+
testhelper.AddCrate(t, path.Join("src", "pubsub"), "google-cloud-pubsub")
311+
if err := command.Run(t.Context(), "git", "add", path.Join("src", "pubsub")); err != nil {
312+
t.Fatal(err)
313+
}
314+
if err := command.Run(t.Context(), "git", "commit", "-m", "feat: created pubsub", "."); err != nil {
315+
t.Fatal(err)
316+
}
317+
if err := Publish(t.Context(), config, true, false); err == nil {
318+
t.Errorf("expected an error publishing with unpushed local commits")
319+
}
320+
}
321+
322+
func TestPublishPreflightError(t *testing.T) {
323+
config := &config.Release{
324+
Preinstalled: map[string]string{
325+
"git": "git-not-found",
326+
},
327+
}
328+
if err := Publish(t.Context(), config, true, false); err == nil {
329+
t.Errorf("expected a preflight error with a bad git command")
330+
}
331+
}
332+
333+
func TestPublishLastTagError(t *testing.T) {
334+
const echo = "/bin/echo"
335+
testhelper.RequireCommand(t, "git")
336+
testhelper.RequireCommand(t, echo)
337+
config := config.Release{
338+
Remote: "origin",
339+
Branch: "invalid-branch",
340+
Preinstalled: map[string]string{
341+
"cargo": echo,
342+
},
343+
}
344+
remoteDir := testhelper.SetupRepoWithChange(t, "release-2001-02-03")
345+
testhelper.CloneRepository(t, remoteDir)
346+
if err := Publish(t.Context(), &config, true, false); err == nil {
347+
t.Fatalf("expected an error during GetLastTag")
348+
}
349+
}

internal/sidekick/rust_release/publish.go

Lines changed: 0 additions & 45 deletions
This file was deleted.

internal/sidekick/rust_release/publish_test.go

Lines changed: 0 additions & 108 deletions
This file was deleted.

0 commit comments

Comments
 (0)