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.

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

feat: add info command #101

Merged
merged 43 commits into from
Sep 27, 2024
Merged

Conversation

rebornplusplus
Copy link
Member

@rebornplusplus rebornplusplus commented Oct 19, 2023

This PR adds a new info command which shows detailed information about package slices.

It accepts a white-space-separated list of strings. The list can be composed of package names, slice names, or a combination of both. The default output format is YAML. When multiple package or slice names are provided, the output is a list of YAML documents, separated by three dashes (“---”).

Slice definitions are shown verbatim according to their definition in the Chisel releases. For example, "globs" are not expanded.

Usage:
  chisel info [info-OPTIONS] [<pkg|slice>...]

[info command options]
      --release=<branch|dir>      Chisel release branch or directory

Expand to see full specification

Command

chisel info

Usage

chisel info [info-OPTIONS] [<pkg|slice>...]

Description

The info command shows detailed information about package slices in a Chisel release. It accepts a white-space separated list of strings. The list can be composed of package names, slice names, or a combination of both.The package slices are then looked for within the upstream chisel-releases repository.

The default output format is YAML. When multiple package/slice names are provided, the output is a list of YAML documents, separated by three dashes (“---”).

  • If no pkgs/slices are provided, it returns an error. At least one pkg/slicename is required.

  • If one or more pkgs/slices are provided, it returns a list of their slice definitions.

  • For slice names, it returns the corresponding package’s SDF, excluding any other package slices which have not been requested.

  • If multiple slices of the same package are provided, it returns the corresponding package’s SDF just once (i.e. the output can only have one YAML document per package, regardless of how many slices are specified).

  • If not all pkgs/slices can be found, it returns a list with slice definitions AND an error (to stderr), listing the pkgs/slices that could not be found.

  • If no packages can be found, it returns an error.

  • Slice definitions are shown verbatim according to their definition in the Chisel releases. E.g. expressions like globs are not expanded.

[info-OPTIONS]

--release <ubuntu-release|dir> 

(Optional) The --release option filters the search. It behaves similar to the --release option to chisel cut. If unspecified, the ubuntu release will be parsed from the host OS. If an ubuntu-release is passed as the argument, the find command only searches for slices in that release branch. If a path to a chisel-release directory is passed as the argument, it parses the directory as a chisel-release and only searches for available slices in that directory.


Examples

$ chisel info openssl_config libssl3_libs 2>/dev/null
package: openssl
archive: ubuntu
slices:
    config:
        contents:
            /etc/ssl/openssl.cnf: {}
            /etc/ssl/private/: {}
            /usr/lib/ssl/certs: {}
            /usr/lib/ssl/openssl.cnf: {}
            /usr/lib/ssl/private: {}
---
package: libssl3
archive: ubuntu
slices:
    libs:
        essential:
            - libc6_libs
        contents:
            /usr/lib/*-linux-*/engines-3/afalg.so: {}
            /usr/lib/*-linux-*/engines-3/loader_attic.so: {}
            /usr/lib/*-linux-*/engines-3/padlock.so: {}
            /usr/lib/*-linux-*/libcrypto.so.*: {}
            /usr/lib/*-linux-*/libssl.so.*: {}
            /usr/lib/*-linux-*/ossl-modules/legacy.so: {}
$ chisel info foo_bar
2023/10/20 11:48:23 Consulting release repository...
2023/10/20 11:48:24 Cached ubuntu-22.04 release is still up-to-date.
2023/10/20 11:48:24 Processing ubuntu-22.04 release...
error: no slice definitions found for: "foo_bar"

Additional information


  • Have you signed the CLA?

Previously, release info (setup.Release) parsing/reading logic was
written in cmd/chisel/cmd_cut.go file. Being the only operation command
before, it made sense to have it there. But, since there are incoming
new commands, it is likely that this logic will be re-used. Thus, move
it to a function, ``getRelease()``.
This commit adds functionalities to produce YAML outputs for
``setup.Package`` and ``setup.Slice`` objects.
This commit adds a new ``info`` command which shows detailed information
about package slices.

It accepts a white-space separated list of strings. The list can be
composed of package names, slice names, or a combination of both. The
default output format is YAML. When multiple package or slice names are
provided, the output is a list of YAML documents, separated by three
dashes (“---”).

Slice definitions are shown verbatim according to their definition in
the Chisel releases. For example, "globs" are not expanded.

---

Usage:
  chisel info [info-OPTIONS] [<pkg|slice>...]

[info command options]
      --release=<branch|dir>      Chisel release branch or directory
@rebornplusplus
Copy link
Member Author

ping @cjdcordeiro @woky @niemeyer

Copy link
Contributor

@woky woky left a comment

Choose a reason for hiding this comment

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

Can't find anything to nitpick on. LGTM.

Copy link

@benhoyt benhoyt left a comment

Choose a reason for hiding this comment

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

Looks good. Just a few very nitty comments/suggestions.

cmd/chisel/cmd_info.go Outdated Show resolved Hide resolved
cmd/chisel/cmd_info.go Outdated Show resolved Hide resolved
cmd/chisel/cmd_info.go Outdated Show resolved Hide resolved
cmd/chisel/cmd_info.go Outdated Show resolved Hide resolved
internal/setup/setup.go Outdated Show resolved Hide resolved
internal/setup/setup.go Show resolved Hide resolved
internal/setup/setup.go Outdated Show resolved Hide resolved
internal/setup/setup.go Outdated Show resolved Hide resolved
internal/setup/setup.go Outdated Show resolved Hide resolved
internal/setup/setup.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@cjdcordeiro cjdcordeiro left a comment

Choose a reason for hiding this comment

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

Noice :)

It lgtm but I'm just leaving a bunch of nitpicks :p

cmd/chisel/cmd_help.go Outdated Show resolved Hide resolved
cmd/chisel/cmd_info.go Outdated Show resolved Hide resolved
cmd/chisel/cmd_info.go Outdated Show resolved Hide resolved
cmd/chisel/cmd_info.go Outdated Show resolved Hide resolved
cmd/chisel/cmd_info_test.go Outdated Show resolved Hide resolved
@cjdcordeiro cjdcordeiro mentioned this pull request Oct 20, 2023
1 task
Copy link
Collaborator

@cjdcordeiro cjdcordeiro left a comment

Choose a reason for hiding this comment

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

Great, thanks for the changes. it lgtm

Copy link
Collaborator

@letFunny letFunny left a comment

Choose a reason for hiding this comment

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

Code looks very good. I left some small, mostly nitpicky, comments.

cmd/chisel/cmd_info.go Outdated Show resolved Hide resolved
cmd/chisel/cmd_cut.go Outdated Show resolved Hide resolved
cmd/chisel/cmd_info.go Outdated Show resolved Hide resolved
cmd/chisel/cmd_info.go Show resolved Hide resolved
cmd/chisel/cmd_info.go Outdated Show resolved Hide resolved
cmd/chisel/cmd_info_test.go Outdated Show resolved Hide resolved
internal/setup/setup.go Outdated Show resolved Hide resolved
internal/setup/setup_test.go Outdated Show resolved Hide resolved
internal/setup/setup_test.go Outdated Show resolved Hide resolved
internal/setup/setup.go Outdated Show resolved Hide resolved
cmd/chisel/helpers.go Outdated Show resolved Hide resolved
Instead of checking if the output YAML equals exactly to the expected
YAML by string checking, parse both YAMLs instead. Then compare to see
if they are equal.
Copy link
Collaborator

@letFunny letFunny left a comment

Choose a reason for hiding this comment

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

Some more minor comments

internal/setup/setup.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@letFunny letFunny left a comment

Choose a reason for hiding this comment

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

Thanks for this Rafid, LGTM, only a couple of nitpicks based on the latest changes and what we discussed.

tests/info/task.yaml Outdated Show resolved Hide resolved
cmd/chisel/cmd_info_test.go Outdated Show resolved Hide resolved
cmd/chisel/cmd_info_test.go Outdated Show resolved Hide resolved
cmd/chisel/cmd_info_test.go Show resolved Hide resolved
cmd/chisel/cmd_info_test.go Outdated Show resolved Hide resolved
internal/setup/setup.go Show resolved Hide resolved
cmd/chisel/cmd_info.go Outdated Show resolved Hide resolved
internal/setup/setup.go Outdated Show resolved Hide resolved
@letFunny letFunny removed the Blocked Waiting for something external label Aug 6, 2024
@rebornplusplus rebornplusplus added the Priority Look at me first label Aug 30, 2024
Copy link
Contributor

@niemeyer niemeyer left a comment

Choose a reason for hiding this comment

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

Thank you all. This is looking pretty good already, but a few details need addressing:

cmd/chisel/cmd_info.go Outdated Show resolved Hide resolved
cmd/chisel/cmd_info.go Outdated Show resolved Hide resolved
cmd/chisel/cmd_info.go Outdated Show resolved Hide resolved
cmd/chisel/cmd_info.go Show resolved Hide resolved
internal/setup/setup.go Outdated Show resolved Hide resolved
internal/setup/setup.go Outdated Show resolved Hide resolved
internal/setup/setup.go Show resolved Hide resolved
internal/setup/setup_test.go Outdated Show resolved Hide resolved
internal/setup/setup_test.go Show resolved Hide resolved
internal/setup/setup_test.go Show resolved Hide resolved
@letFunny letFunny added the Blocked Waiting for something external label Sep 17, 2024
@letFunny letFunny removed the Blocked Waiting for something external label Sep 20, 2024
Copy link
Contributor

@niemeyer niemeyer left a comment

Choose a reason for hiding this comment

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

This is looking good. Only remaining question is the mutability question that was already in the previous review. It changed, but it's not clear what is the real intention or the current state yet.

internal/setup/setup.go Outdated Show resolved Hide resolved
internal/setup/setup_test.go Outdated Show resolved Hide resolved
internal/setup/setup_test.go Outdated Show resolved Hide resolved
@letFunny letFunny added the Blocked Waiting for something external label Sep 24, 2024
Copy link
Collaborator

@letFunny letFunny left a comment

Choose a reason for hiding this comment

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

Thanks Rafid, one minor thing.

internal/setup/setup.go Outdated Show resolved Hide resolved
internal/setup/setup_test.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@letFunny letFunny left a comment

Choose a reason for hiding this comment

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

Last minor things in the same spirit as one of the comments by Gustavo

internal/setup/setup_test.go Outdated Show resolved Hide resolved
internal/setup/setup_test.go Outdated Show resolved Hide resolved
@letFunny letFunny removed the Blocked Waiting for something external label Sep 25, 2024
@letFunny letFunny requested a review from niemeyer September 26, 2024 16:08
Copy link
Contributor

@niemeyer niemeyer left a comment

Choose a reason for hiding this comment

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

LGTM, thank you both.

@niemeyer niemeyer merged commit 247bdb9 into canonical:main Sep 27, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority Look at me first
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants