Skip to content

Commit bf51454

Browse files
tobiasb-msCopilottobiasb_microsoft
authored
feat: add --local-repo-with-publish to create and publish to local repo during build (#412)
* feat: add --local-repo-with-publish to create and publish to local repo during build * fix: prevent possibly bad copy when --local-repo-with-publish is our build directory ./base/out * refactor: use Cobra's native MarkFlagsMutuallyExclusive for flag validation (#421) * Initial plan * refactor: use Cobra's native MarkFlagsMutuallyExclusive for --srpm-only and --local-repo-with-publish Co-authored-by: tobiasb_microsoft <115835401+tobiasb_microsoft@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: tobiasb_microsoft <115835401+tobiasb_microsoft@users.noreply.github.com> * refactor: convert --local-repo-with-publish to single-use argument (#422) * Initial plan * refactor: convert --local-repo-with-publish to single-use argument Co-authored-by: tobiasb_microsoft <115835401+tobiasb_microsoft@users.noreply.github.com> * refactor: convert --local-repo-with-publish to single-use argument Co-authored-by: tobiasb_microsoft <115835401+tobiasb_microsoft@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: tobiasb_microsoft <115835401+tobiasb_microsoft@users.noreply.github.com> * refactor: move publishing logic into buildComponentUsingBuilder for thin CLI wrapper (#425) * Initial plan * refactor: move publishing logic into buildComponentUsingBuilder Move RPM publishing logic from BuildComponent into buildComponentUsingBuilder to make the CLI command a thin wrapper. BuildComponent now simply sets up dependencies and calls buildComponentUsingBuilder, which handles all the actual build work including publishing. This addresses review feedback to keep CLI command implementations lightweight. Co-authored-by: tobiasb_microsoft <115835401+tobiasb_microsoft@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: tobiasb_microsoft <115835401+tobiasb_microsoft@users.noreply.github.com> * refactor: Move local repo publish logic to NewPublisher (#428) * Initial plan * refactor: move createrepo_c check and initialization into NewPublisher Simplifies the NewPublisher API by: - Accepting opctx.Ctx instead of individual interfaces - Adding an `initialize` parameter to optionally check for createrepo_c and initialize the repo - Removing the need for callers to manually check RequireCreaterepoC and call initializePublishRepo This addresses feedback to move local repo publish logic into NewPublisher rather than requiring callers to know about these details. Co-authored-by: tobiasb_microsoft <115835401+tobiasb_microsoft@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: tobiasb_microsoft <115835401+tobiasb_microsoft@users.noreply.github.com> --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: tobiasb_microsoft <115835401+tobiasb_microsoft@users.noreply.github.com>
1 parent b86d665 commit bf51454

7 files changed

Lines changed: 542 additions & 163 deletions

File tree

internal/app/azldev/cmds/component/build.go

Lines changed: 110 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package component
66
import (
77
"errors"
88
"fmt"
9+
"path/filepath"
910

1011
"github.com/gim-home/azldev-preview/internal/app/azldev"
1112
"github.com/gim-home/azldev-preview/internal/app/azldev/core/componentbuilder"
@@ -16,6 +17,7 @@ import (
1617
"github.com/gim-home/azldev-preview/internal/providers/sourceproviders"
1718
"github.com/gim-home/azldev-preview/internal/utils/defers"
1819
"github.com/gim-home/azldev-preview/internal/utils/fileutils"
20+
"github.com/gim-home/azldev-preview/internal/utils/localrepo"
1921
"github.com/spf13/cobra"
2022
)
2123

@@ -27,7 +29,8 @@ type ComponentBuildOptions struct {
2729
SourcePackageOnly bool
2830
BuildEnvPolicy BuildEnvPreservePolicy
2931

30-
LocalRepoPaths []string
32+
LocalRepoPaths []string
33+
LocalRepoWithPublishPath string
3134
}
3235

3336
// ComponentBuildResults summarizes the results of building a single component.
@@ -76,12 +79,22 @@ func NewBuildCmd() *cobra.Command {
7679
))
7780
cmd.Flags().StringArrayVar(&options.LocalRepoPaths, "local-repo", []string{},
7881
"Paths to local repositories to include during build (can be specified multiple times)")
82+
cmd.Flags().StringVar(&options.LocalRepoWithPublishPath, "local-repo-with-publish", "",
83+
"Path to local repository to include during build and publish built RPMs to")
84+
85+
// Mark flags as mutually exclusive.
86+
cmd.MarkFlagsMutuallyExclusive("srpm-only", "local-repo-with-publish")
7987

8088
return cmd
8189
}
8290

8391
func SelectAndBuildComponents(env *azldev.Env, options *ComponentBuildOptions,
8492
) ([]ComponentBuildResults, error) {
93+
// Validate options before doing any work.
94+
if err := validateBuildOptions(env, options); err != nil {
95+
return nil, err
96+
}
97+
8598
var comps *components.ComponentSet
8699

87100
resolver := components.NewResolver(env)
@@ -173,9 +186,22 @@ func BuildComponent(
173186

174187
builder := componentbuilder.New(env, env.FS(), env, sourcePreparer, buildEnv, workDirFactory)
175188

176-
return buildComponentUsingBuilder(env, component, builder, options.LocalRepoPaths,
189+
// Combine both local repo paths for use during build (both are used as dependencies).
190+
allLocalRepoPaths := append([]string{}, options.LocalRepoPaths...)
191+
if options.LocalRepoWithPublishPath != "" {
192+
allLocalRepoPaths = append(allLocalRepoPaths, options.LocalRepoWithPublishPath)
193+
}
194+
195+
results, err = buildComponentUsingBuilder(
196+
env, component, builder, allLocalRepoPaths,
177197
options.SourcePackageOnly, options.RunChecks,
198+
options.LocalRepoWithPublishPath,
178199
)
200+
if err != nil {
201+
return results, err
202+
}
203+
204+
return results, nil
179205
}
180206

181207
func buildComponentUsingBuilder(
@@ -184,6 +210,7 @@ func buildComponentUsingBuilder(
184210
builder *componentbuilder.Builder,
185211
localRepoPaths []string,
186212
sourcePackageOnly, runChecks bool,
213+
localRepoWithPublishPath string,
187214
) (results ComponentBuildResults, err error) {
188215
// Compose the path to the output dir.
189216
outputDir := env.OutputDir()
@@ -227,5 +254,86 @@ func buildComponentUsingBuilder(
227254
return results, fmt.Errorf("failed to build RPM for %q: %w", component.GetName(), err)
228255
}
229256

257+
// Publish built RPMs to local repo with publish enabled.
258+
if localRepoWithPublishPath != "" && len(results.RPMPaths) > 0 {
259+
publishErr := publishToLocalRepo(env, results.RPMPaths, localRepoWithPublishPath)
260+
if publishErr != nil {
261+
return results, fmt.Errorf("failed to publish RPMs for %q:\n%w", component.GetName(), publishErr)
262+
}
263+
}
264+
230265
return results, nil
231266
}
267+
268+
// validateBuildOptions validates the build options before any work is done.
269+
func validateBuildOptions(env *azldev.Env, options *ComponentBuildOptions) error {
270+
// Check for overlap between --local-repo and --local-repo-with-publish.
271+
// (Check config errors before tool availability for better UX.)
272+
if err := checkLocalRepoPathOverlap(options.LocalRepoPaths, options.LocalRepoWithPublishPath); err != nil {
273+
return err
274+
}
275+
276+
// If we have a repo to publish to, create and initialize it.
277+
// This checks for createrepo_c availability and initializes the repo metadata
278+
// so it can be used as a dependency source during builds.
279+
if options.LocalRepoWithPublishPath != "" {
280+
_, err := localrepo.NewPublisher(env, options.LocalRepoWithPublishPath, true)
281+
if err != nil {
282+
return fmt.Errorf("failed to initialize publish repository:\n%w", err)
283+
}
284+
}
285+
286+
return nil
287+
}
288+
289+
// checkLocalRepoPathOverlap checks that the path doesn't appear in both --local-repo and --local-repo-with-publish.
290+
func checkLocalRepoPathOverlap(localRepoPaths []string, localRepoWithPublishPath string) error {
291+
// If no publish path is specified, there's no overlap.
292+
if localRepoWithPublishPath == "" {
293+
return nil
294+
}
295+
296+
// Normalize the publish path for comparison.
297+
absPublishPath, err := filepath.Abs(localRepoWithPublishPath)
298+
if err != nil {
299+
return fmt.Errorf("failed to resolve absolute path for %#q:\n%w", localRepoWithPublishPath, err)
300+
}
301+
302+
// Check if the publish path appears in local repo paths.
303+
for _, repoPath := range localRepoPaths {
304+
absPath, err := filepath.Abs(repoPath)
305+
if err != nil {
306+
return fmt.Errorf("failed to resolve absolute path for %#q:\n%w", repoPath, err)
307+
}
308+
309+
if absPath == absPublishPath {
310+
return fmt.Errorf(
311+
"path %#q appears in both --local-repo and --local-repo-with-publish; "+
312+
"use --local-repo-with-publish only for repos you want to both read from and publish to",
313+
localRepoWithPublishPath,
314+
)
315+
}
316+
}
317+
318+
return nil
319+
}
320+
321+
// publishToLocalRepo publishes the given RPMs to the specified local repo.
322+
func publishToLocalRepo(env *azldev.Env, rpmPaths []string, repoPath string) error {
323+
publisher, err := localrepo.NewPublisher(env, repoPath, false)
324+
if err != nil {
325+
return fmt.Errorf("failed to create publisher for %#q:\n%w", repoPath, err)
326+
}
327+
328+
// Ensure the repo directory exists.
329+
if err := publisher.EnsureRepoExists(); err != nil {
330+
return fmt.Errorf("ensuring local repo exists:\n%w", err)
331+
}
332+
333+
// Publish RPMs and update repo metadata.
334+
if err := publisher.PublishRPMs(env, rpmPaths); err != nil {
335+
return fmt.Errorf("failed to publish to %#q:\n%w", repoPath, err)
336+
}
337+
338+
return nil
339+
}

internal/app/azldev/cmds/component/build_test.go

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,3 +40,49 @@ func TestBuildComponents_NoComponents(t *testing.T) {
4040
require.Error(t, err)
4141
require.Empty(t, results)
4242
}
43+
44+
func TestValidateBuildOptions_SRPMOnlyWithPublish(t *testing.T) {
45+
testEnv := testutils.NewTestEnv(t)
46+
47+
cmd := testutils.PrepareCommand(componentcmds.NewBuildCmd(), testEnv.Env)
48+
cmd.SetArgs([]string{"--srpm-only", "--local-repo-with-publish=/some/repo"})
49+
err := cmd.Execute()
50+
51+
require.Error(t, err)
52+
assert.Contains(
53+
t, err.Error(),
54+
"if any flags in the group [srpm-only local-repo-with-publish] are set none of the others can be",
55+
)
56+
}
57+
58+
func TestValidateBuildOptions_PathOverlap(t *testing.T) {
59+
testEnv := testutils.NewTestEnv(t)
60+
61+
options := componentcmds.ComponentBuildOptions{
62+
LocalRepoPaths: []string{"/shared/repo"},
63+
LocalRepoWithPublishPath: "/shared/repo",
64+
}
65+
66+
_, err := componentcmds.SelectAndBuildComponents(testEnv.Env, &options)
67+
68+
require.Error(t, err)
69+
assert.Contains(t, err.Error(), "appears in both --local-repo and --local-repo-with-publish")
70+
}
71+
72+
func TestValidateBuildOptions_NoOverlapWithDifferentPaths(t *testing.T) {
73+
testEnv := testutils.NewTestEnv(t)
74+
75+
// This should not fail validation due to path overlap.
76+
// It will fail due to createrepo_c not being available (which is expected in unit tests).
77+
options := componentcmds.ComponentBuildOptions{
78+
LocalRepoPaths: []string{"/upstream/repo"},
79+
LocalRepoWithPublishPath: "/dev/repo",
80+
}
81+
82+
_, err := componentcmds.SelectAndBuildComponents(testEnv.Env, &options)
83+
84+
// Should fail because createrepo_c is not available, NOT because of path overlap.
85+
require.Error(t, err)
86+
assert.Contains(t, err.Error(), "createrepo_c")
87+
assert.NotContains(t, err.Error(), "appears in both")
88+
}

internal/utils/fileutils/file.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,3 +42,10 @@ func Exists(fs opctx.FS, path string) (bool, error) {
4242
//nolint:wrapcheck // We are intentionally a pass-through.
4343
return afero.Exists(fs, path)
4444
}
45+
46+
// Adaptation of [afero.DirExists] that works with [opctx.FS].
47+
// Checks if a directory exists at the given path.
48+
func DirExists(fs opctx.FS, path string) (bool, error) {
49+
//nolint:wrapcheck // We are intentionally a pass-through.
50+
return afero.DirExists(fs, path)
51+
}

0 commit comments

Comments
 (0)