-
Notifications
You must be signed in to change notification settings - Fork 26
fix: support upstream specs in component query #110
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,10 +5,15 @@ package component | |
|
|
||
| import ( | ||
| "fmt" | ||
| "path/filepath" | ||
|
|
||
| "github.com/microsoft/azure-linux-dev-tools/internal/app/azldev" | ||
| "github.com/microsoft/azure-linux-dev-tools/internal/app/azldev/core/components" | ||
| "github.com/microsoft/azure-linux-dev-tools/internal/app/azldev/core/sources" | ||
| "github.com/microsoft/azure-linux-dev-tools/internal/app/azldev/core/specs" | ||
| "github.com/microsoft/azure-linux-dev-tools/internal/app/azldev/core/workdir" | ||
| "github.com/microsoft/azure-linux-dev-tools/internal/projectconfig" | ||
| "github.com/microsoft/azure-linux-dev-tools/internal/providers/sourceproviders" | ||
| "github.com/spf13/cobra" | ||
| ) | ||
|
|
||
|
|
@@ -58,6 +63,15 @@ type componentDetails struct { | |
| specs.ComponentSpecDetails | ||
| } | ||
|
|
||
| type queryComponent struct { | ||
| components.Component | ||
| config projectconfig.ComponentConfig | ||
| } | ||
|
|
||
| func (c *queryComponent) GetConfig() *projectconfig.ComponentConfig { | ||
| return &c.config | ||
| } | ||
|
|
||
| // Queries env for component details, in accordance with options. Returns the found components. | ||
| func QueryComponents( | ||
| env *azldev.Env, options *QueryComponentsOptions, | ||
|
|
@@ -74,9 +88,7 @@ func QueryComponents( | |
| allDetails := make([]*componentDetails, 0, comps.Len()) | ||
|
|
||
| for _, comp := range comps.Components() { | ||
| spec := comp.GetSpec() | ||
|
|
||
| specInfo, err := spec.Parse() | ||
| specInfo, err := parseComponentSpec(env, comp) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to parse spec for component %q:\n%w", comp.GetName(), err) | ||
| } | ||
|
|
@@ -90,3 +102,54 @@ func QueryComponents( | |
|
|
||
| return allDetails, nil | ||
| } | ||
|
|
||
| func parseComponentSpec(env *azldev.Env, comp components.Component) (*specs.ComponentSpecDetails, error) { | ||
| if comp.GetConfig().Spec.SourceType == projectconfig.SpecSourceTypeLocal { | ||
| return comp.GetSpec().Parse() | ||
| } | ||
|
|
||
| componentForPrep := comp | ||
| if comp.GetConfig().Spec.SourceType == projectconfig.SpecSourceTypeUnspecified { | ||
| normalizedConfig := *comp.GetConfig() | ||
| normalizedConfig.Spec.SourceType = projectconfig.SpecSourceTypeUpstream | ||
| componentForPrep = &queryComponent{ | ||
| Component: comp, | ||
| config: normalizedConfig, | ||
| } | ||
| } | ||
|
|
||
| distro, err := sourceproviders.ResolveDistro(env, componentForPrep) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to resolve distro for component %q:\n%w", comp.GetName(), err) | ||
| } | ||
|
|
||
| sourceManager, err := sourceproviders.NewSourceManager(env, distro) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to create source manager:\n%w", err) | ||
| } | ||
|
|
||
| preparer, err := sources.NewPreparer(sourceManager, env.FS(), env, env, sources.WithSkipLookaside()) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to create source preparer:\n%w", err) | ||
| } | ||
|
|
||
| workDirFactory, err := workdir.NewFactory(env.FS(), env.WorkDir(), env.ConstructionTime()) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to create work dir factory:\n%w", err) | ||
| } | ||
|
|
||
| preparedSourcesDir, err := workDirFactory.Create(comp.GetName(), "query-spec") | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to create work dir for component %#q:\n%w", comp.GetName(), err) | ||
| } | ||
|
|
||
| if err := preparer.PrepareSources(env, componentForPrep, preparedSourcesDir, true /* applyOverlays */); err != nil { | ||
| return nil, fmt.Errorf("failed to prepare sources for component %#q:\n%w", comp.GetName(), err) | ||
|
Comment on lines
+131
to
+147
|
||
| } | ||
|
|
||
| preparedConfig := *componentForPrep.GetConfig() | ||
| preparedConfig.Spec.SourceType = projectconfig.SpecSourceTypeLocal | ||
| preparedConfig.Spec.Path = filepath.Join(preparedSourcesDir, comp.GetName()+".spec") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The hardcoded |
||
|
|
||
| return specs.NewSpec(env, preparedConfig).Parse() | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -80,3 +80,86 @@ func TestQueryComponents_OneComponent(t *testing.T) { | |
| result := results[0] | ||
| assert.Equal(t, testComponentName, result.Name) | ||
| } | ||
|
|
||
| func TestQueryComponents_UpstreamComponent(t *testing.T) { | ||
| const testComponentName = "test-component" | ||
| const testUpstreamName = "test-component-upstream" | ||
|
|
||
| testCases := []struct { | ||
| name string | ||
| component projectconfig.ComponentConfig | ||
| specFileName string | ||
| specNameField string | ||
| }{ | ||
| { | ||
| name: "default spec source type uses normalized component name", | ||
| component: projectconfig.ComponentConfig{ | ||
| Name: testComponentName, | ||
| }, | ||
| specFileName: testComponentName, | ||
| specNameField: testComponentName, | ||
| }, | ||
| { | ||
| name: "explicit upstream spec source type uses upstream name", | ||
| component: projectconfig.ComponentConfig{ | ||
| Name: testComponentName, | ||
| Spec: projectconfig.SpecConfig{ | ||
| SourceType: projectconfig.SpecSourceTypeUpstream, | ||
| UpstreamName: testUpstreamName, | ||
| }, | ||
| }, | ||
|
Comment on lines
+104
to
+110
|
||
| specFileName: testUpstreamName, | ||
| specNameField: testUpstreamName, | ||
| }, | ||
| } | ||
|
|
||
| for _, testCase := range testCases { | ||
| t.Run(testCase.name, func(t *testing.T) { | ||
| testEnv := testutils.NewTestEnv(t) | ||
| testEnv.Config.Components[testComponentName] = testCase.component | ||
|
|
||
| testEnv.CmdFactory.RegisterCommandInSearchPath(mock.MockBinary) | ||
|
|
||
| testEnv.CmdFactory.RunHandler = func(cmd *exec.Cmd) error { | ||
| if len(cmd.Args) >= 2 && cmd.Args[0] == "git" && cmd.Args[1] == "clone" { | ||
| cloneDir := cmd.Args[len(cmd.Args)-1] | ||
| specPath := cloneDir + "/" + testCase.specFileName + ".spec" | ||
|
|
||
| return fileutils.WriteFile( | ||
| testEnv.FS(), | ||
| specPath, | ||
| []byte("Name: "+testCase.specNameField+"\nVersion: 1.0.0\n"), | ||
| fileperms.PublicFile, | ||
| ) | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| testEnv.CmdFactory.RunAndGetOutputHandler = func(cmd *exec.Cmd) (string, error) { | ||
| if len(cmd.Args) >= 5 && | ||
| cmd.Args[0] == "git" && | ||
| cmd.Args[1] == "-C" && | ||
| cmd.Args[3] == "rev-parse" && | ||
| cmd.Args[4] == "HEAD" { | ||
| return "head123abc\n", nil | ||
| } | ||
|
|
||
| return "name=test-component\nepoch=0\nversion=1.0.0\nrelease=1.azl3\n", nil | ||
| } | ||
|
Comment on lines
+139
to
+149
|
||
|
|
||
| options := component.QueryComponentsOptions{ | ||
| ComponentFilter: components.ComponentFilter{ | ||
| ComponentNamePatterns: []string{testComponentName}, | ||
| }, | ||
| } | ||
|
|
||
| results, err := component.QueryComponents(testEnv.Env, &options) | ||
| require.NoError(t, err) | ||
| require.Len(t, results, 1) | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given the work-dir leak noted in |
||
| result := results[0] | ||
| assert.Equal(t, testComponentName, result.Name) | ||
| }) | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[blocking]: The prepared work dir is leaked.
workDirFactory.Createreturns a real directory underWorkDir()and doesn't self-clean, and this function never removespreparedSourcesDir. Sincequeryruns this per component in the loop, every invocation leaves one clone/prepared tree per non-local component behind, growing without bound.render.gopairs its staging dir with a deferred cleanup and the same cleanup can be done here: