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

Enable libexec artifacts #408

Merged
merged 9 commits into from
Oct 23, 2024
Merged

Enable libexec artifacts #408

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
f930ca4
Enable libexec artifacts
pmengelbert Oct 21, 2024
049dd28
Add libexec capability for debian-based distros
pmengelbert Oct 21, 2024
b334181
Add documentation for libexec artifacts
pmengelbert Oct 21, 2024
d832058
Add tests for libexec
pmengelbert Oct 21, 2024
4ac9745
Move subpath defaulting for libexec artifacts
pmengelbert Oct 22, 2024
dcbeca1
Address PR comments
pmengelbert Oct 22, 2024
0d4ecc2
Run `go generate`
pmengelbert Oct 22, 2024
0d1a89e
Don't insert name in path for libexec artifacts
pmengelbert Oct 23, 2024
fa22e90
Update docs
pmengelbert Oct 23, 2024
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
5 changes: 5 additions & 0 deletions artifacts.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import (
type Artifacts struct {
// Binaries is the list of binaries to include in the package.
Binaries map[string]ArtifactConfig `yaml:"binaries,omitempty" json:"binaries,omitempty"`
// Libexec is the list of additional binaries that may be invoked by the main package binary.
Libexec map[string]ArtifactConfig `yaml:"libexec,omitempty" json:"libexec,omitempty"`
// Manpages is the list of manpages to include in the package.
Manpages map[string]ArtifactConfig `yaml:"manpages,omitempty" json:"manpages,omitempty"`
// DataDirs is a list of read-only architecture-independent data files, to be placed in /usr/share/
Expand Down Expand Up @@ -95,6 +97,9 @@ type ArtifactConfig struct {
// Name is file or dir name to use for the artifact in the package.
// If empty, the file or dir name from the produced artifact will be used.
Name string `yaml:"name,omitempty" json:"name,omitempty"`
// PackageName is the name of the package. This can be used to replace
// `spec.Name` when destination paths are generated.
PackageName string `yaml:"packageName,omitempty" json:"packageName,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

My reasoning for suggesting a bool here is because the package name can go into SubPath and we'd just skip including the spec's package name with that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Next time I will clarify what you want before I implement the solution. I thought you meant that either was acceptable

Copy link
Contributor Author

@pmengelbert pmengelbert Oct 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

tldr: I think we should just use subPath and not do anything automatically. With an empty subPath, things go directly into /usr/libexec (which is standard). Otherwise, you can set subPath to docker if you want things to go into /usr/libexec/docker or docker/cli-plugins if you want /usr/libexec/docker/cli-plugins. Detailed explanation follows.

My reasoning for suggesting a bool here is because the package name can go into SubPath and we'd just skip including the spec's package name with that.

So then in order to get the desired behavior in the example I've been using, you would need

name: moby-buildx

artifacts:
  libexec:
    bin/docker-buildx:
      subPath: docker/cli-plugins
      omitAutoPackageName: true

What I don't like about that is it kind of overloads the subPath field. When we don't need to change the package name, it would be

name: docker

artifacts:
  libexec:
    bin/docker-buildx:
      subPath: cli-plugins

Both with the bool and without, it's a leaky abstraction. The user has to know several things:

  1. the name field from the spec (which is usually at the top of the file, dozens of lines away) is inserted some times but not others (yes in /usr/lib artifacts, no in /usr/bin artifacts)
  2. libexec will be one of the times when the name is inserted
  3. in order to avoid inserting the name you need to set the bool
  4. the bool changes the meaning of the subPath field, and changes the absolute path depth.

It's too much to ask of the user IMO. Better to just have /usr/libexec be the default (since that's a pretty standard thing to do) and have the user add the application name to the subpath if they want things nested in a subdirectory..

Copy link
Member

Choose a reason for hiding this comment

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

I think the issue is its 50/50 (literally on my system it is right at the 50/50 mark).
In this case also I guess the package name may not line up exactly with what's being installed as well (beyond docker cli plugins) since we need to differentiate package names from upstreams.

Ok, it seems fine to just not use the package name in the path here (and add to the docs).
Thanks for the diligence here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

}

func (a *ArtifactConfig) ResolveName(path string) string {
Expand Down
11 changes: 11 additions & 0 deletions docs/spec.schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@
"name": {
"type": "string",
"description": "Name is file or dir name to use for the artifact in the package.\nIf empty, the file or dir name from the produced artifact will be used."
},
"packageName": {
"type": "string",
"description": "PackageName is the name of the package. This can be used to replace\n`spec.Name` when destination paths are generated."
}
},
"additionalProperties": false,
Expand Down Expand Up @@ -76,6 +80,13 @@
"type": "object",
"description": "Binaries is the list of binaries to include in the package."
},
"libexec": {
"additionalProperties": {
"$ref": "#/$defs/ArtifactConfig"
},
"type": "object",
"description": "Libexec is the list of additional binaries that may be invoked by the main package binary."
},
"manpages": {
"additionalProperties": {
"$ref": "#/$defs/ArtifactConfig"
Expand Down
16 changes: 16 additions & 0 deletions frontend/deb/debroot.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,22 @@ func createInstallScripts(worker llb.State, spec *dalec.Spec, dir string) []llb.
}
}

if len(spec.Artifacts.Libexec) > 0 {
sorted := dalec.SortMapKeys(spec.Artifacts.Libexec)
for _, key := range sorted {
cfg := spec.Artifacts.Libexec[key]

packageName := cfg.PackageName
if packageName == "" {
packageName = spec.Name
}

resolved := cfg.ResolveName(key)
targetDir := filepath.Join(`/usr/libexec`, packageName, cfg.SubPath)
writeInstall(key, targetDir, resolved)
}
}

if len(spec.Artifacts.Libs) > 0 {
sorted := dalec.SortMapKeys(spec.Artifacts.Libs)
for _, key := range sorted {
Expand Down
39 changes: 39 additions & 0 deletions frontend/rpm/template.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,30 @@ func (w *specWrapper) Install() fmt.Stringer {
}
}

if w.Spec.Artifacts.Libexec != nil {
cpuguy83 marked this conversation as resolved.
Show resolved Hide resolved
libexecFileKeys := dalec.SortMapKeys(w.Spec.Artifacts.Libexec)
for _, k := range libexecFileKeys {
le := w.Spec.Artifacts.Libexec[k]

packageName := le.PackageName
if packageName == "" {
packageName = w.Spec.Name
}

targetDir := filepath.Join(`%{buildroot}/%{_libexecdir}`, packageName, le.SubPath)
fmt.Fprintln(b, "mkdir -p", targetDir)

var targetPath string
file := le.ResolveName(k)
if !strings.Contains(file, "*") {
targetPath = filepath.Join(targetDir, file)
} else {
targetPath = targetDir + "/"
}
fmt.Fprintln(b, "cp -r", k, targetPath)
}
}

configKeys := dalec.SortMapKeys(w.Spec.Artifacts.ConfigFiles)
for _, c := range configKeys {
cfg := w.Spec.Artifacts.ConfigFiles[c]
Expand Down Expand Up @@ -599,6 +623,21 @@ func (w *specWrapper) Files() fmt.Stringer {
}
}

if w.Spec.Artifacts.Libexec != nil {
dataKeys := dalec.SortMapKeys(w.Spec.Artifacts.Libexec)
for _, k := range dataKeys {
le := w.Spec.Artifacts.Libexec[k]
packageName := le.PackageName
if packageName == "" {
packageName = w.Spec.Name
}

targetDir := filepath.Join(`%{_libexecdir}`, packageName, le.SubPath)
fullPath := filepath.Join(targetDir, le.ResolveName(k))
fmt.Fprintln(b, fullPath)
}
}

configKeys := dalec.SortMapKeys(w.Spec.Artifacts.ConfigFiles)
for _, c := range configKeys {
cfg := w.Spec.Artifacts.ConfigFiles[c]
Expand Down
91 changes: 91 additions & 0 deletions test/azlinux_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1015,6 +1015,97 @@ Environment="KUBELET_KUBECONFIG_ARGS=--bootstrap-kubeconfig=/etc/kubernetes/boot
})
})

t.Run("test libexec file installation", func(t *testing.T) {
t.Parallel()
spec := &dalec.Spec{
Name: "libexec-test",
Version: "0.0.1",
Revision: "1",
License: "MIT",
Website: "https://github.com/azure/dalec",
Vendor: "Dalec",
Packager: "Dalec",
Description: "Should install specified data files",
Sources: map[string]dalec.Source{
"no_name_no_subpath": {
Inline: &dalec.SourceInline{
File: &dalec.SourceInlineFile{
Contents: "#!/usr/bin/env bash\necho hello world",
Permissions: 0o755,
},
},
},
"name_only": {
Inline: &dalec.SourceInline{
File: &dalec.SourceInlineFile{
Contents: "#!/usr/bin/env bash\necho hello world",
Permissions: 0o755,
},
},
},
"name_and_subpath": {
Inline: &dalec.SourceInline{
File: &dalec.SourceInlineFile{
Contents: "#!/usr/bin/env bash\necho hello world",
Permissions: 0o755,
},
},
},
"subpath_only": {
Inline: &dalec.SourceInline{
File: &dalec.SourceInlineFile{
Contents: "#!/usr/bin/env bash\necho hello world",
Permissions: 0o755,
},
},
},
},
Build: dalec.ArtifactBuild{},
Artifacts: dalec.Artifacts{
Binaries: map[string]dalec.ArtifactConfig{
"no_name_no_subpath": {},
},
Libexec: map[string]dalec.ArtifactConfig{
"no_name_no_subpath": {},
"name_only": {
Name: "name_only",
},
"name_and_subpath": {
SubPath: "subpath",
Name: "custom_name",
PackageName: "custom",
},
"subpath_only": dalec.ArtifactConfig{
SubPath: "custom",
},
},
},
}

testEnv.RunTest(ctx, t, func(ctx context.Context, client gwclient.Client) {
req := newSolveRequest(withBuildTarget(testConfig.Target.Container), withSpec(ctx, t, spec))
res := solveT(ctx, t, client, req)

ref, err := res.SingleRef()
if err != nil {
t.Fatal(err)
}

if err := validatePathAndPermissions(ctx, ref, "/usr/libexec/libexec-test/no_name_no_subpath", 0o755); err != nil {
t.Fatal(err)
}
if err := validatePathAndPermissions(ctx, ref, "/usr/libexec/libexec-test/name_only", 0o755); err != nil {
t.Fatal(err)
}
if err := validatePathAndPermissions(ctx, ref, "/usr/libexec/custom/subpath/custom_name", 0o755); err != nil {
t.Fatal(err)
}
if err := validatePathAndPermissions(ctx, ref, "/usr/libexec/libexec-test/custom/subpath_only", 0o755); err != nil {
t.Fatal(err)
}
})
})

t.Run("test config files handled", func(t *testing.T) {
t.Parallel()
spec := &dalec.Spec{
Expand Down
26 changes: 26 additions & 0 deletions website/docs/artifacts.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,32 @@ artifacts:
You may use a trailing wildcard to specify multiple binaries in a directory,
though behavior may differ between different OS's/distros.

### Libexec

Libexec files are additional executable files that may be executed by one of
the main package executables. On Linux these would typically get installed into
`/usr/libexec/<package-name>`.

Files under libexec are a mapping of file path to [artifact configuration](#artifact-configuration).
If `subpath` is not supplied, it will default to the package name. The file
path is the path to a file that must be available after the build section has
finished. This path is relative to the working directory of the build phase
*before* any directory changes are made.

Example:

```yaml
name: my_package

artifacts:
# the following config will install my_bin at /usr/libexec/my package/my_bin
libexec:
src/my_bin:
```

You may use a trailing wildcard to specify multiple binaries in a directory,
though behavior may differ between different OS's/distros.

### Manpages

Manpages is short for manual pages.
Expand Down