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

Improve inspect-builder output #769

Merged
merged 8 commits into from
Aug 11, 2020

Conversation

dwillist
Copy link
Contributor

  • by default show all id/version information for nested buildpacks
  • allow a '--depth' or '-d' flag to truncate this nesting at a configurable depth
  • clean up the 'Buildpacks:' list to omit repetitions with the same buildpack id and version

Signed-off-by: dwillist dthornton@vmware.com

Summary

This lets the inspect-builder subcommand recursively list all group and buildpack information for a builder.
The old behavior can still be achieved with pack inspect-builder --depth 1

Additionally updates the inspect-builder output under Buildpacks: to omit repetitions.

Output

Before

We did not display Detection Order information for metabuildpacks.

Buildpacks:
  ID                           VERSION                             HOMEPAGE
  read/env                     read-env-version
  noop.buildpack               noop.buildpack.version
  noop.buildpack               noop.buildpack.later-version        http://geocities.com/cool-bp
  simple/layers                simple-layers-version
  simple/nested-level-2        nested-l2-version
  simple/nested-level-1        nested-l1-version
  read/env                     read-env-version

Detection Order:
  Group #1:
    simple/nested-level-1
    read/env@read-env-version                      (optional)

After

We can display Detection order to the desired depth using --depth flag, omission displays everything. Additionally the read/env buildpack entry is no longer repeated.
Example

pack inspect-builder <builder-name>

Buildpacks:
  ID                           VERSION                             HOMEPAGE
  read/env                     read-env-version
  noop.buildpack               noop.buildpack.version
  noop.buildpack               noop.buildpack.later-version        http://geocities.com/cool-bp
  simple/layers                simple-layers-version
  simple/nested-level-2        nested-l2-version
  simple/nested-level-1        nested-l1-version

Detection Order:
  Group #1:
    simple/nested-level-1
      Group #2:
        simple/nested-level-2@nested--version
          Group #3:
            simple/layers@simple-layers-version
            read/env@read-env-version
    read/env@read-env-version                      (optional)

Example using the --depth flag

pack inspect-builder <builder-name> --depth 2

Detection Order:
  Group #1:
    simple/nested-level-1
      Group #2:
        simple/nested-level-2@nested-l2-version
    read/env@read-env-version                      (optional)

Documentation

This will require documenting the new --depth flag that can be passed to inspect-builder

  • Should this change be documented?
    • Yes (additions to this PR coming)
    • No

Related

Resolves #253

- by default show all id/version information for nested buildpacks
- allow a '--depth' or '-d' flag to truncate this nesting at a configurable depth
- clean up the 'Buildpacks:' list to omit repetitions with the same buildpack id and version

Signed-off-by: dwillist <dthornton@vmware.com>
@dwillist dwillist requested a review from a team as a code owner July 30, 2020 20:39
- allow for recursive structure of buildpacks as it is possible to create

Signed-off-by: dwillist <dthornton@vmware.com>
@dwillist dwillist changed the title update the inspect-output to (draft) update inspect-builder output Aug 3, 2020
@dwillist dwillist force-pushed the 253_inspect_builder_enhancement branch 7 times, most recently from 6a551aa to e6f5f5f Compare August 4, 2020 01:26
@codecov
Copy link

codecov bot commented Aug 4, 2020

Codecov Report

Merging #769 into main will increase coverage by 0.20%.
The diff coverage is 85.04%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #769      +/-   ##
==========================================
+ Coverage   74.76%   74.96%   +0.20%     
==========================================
  Files          77       77              
  Lines        5174     5274     +100     
==========================================
+ Hits         3868     3953      +85     
- Misses       1001     1011      +10     
- Partials      305      310       +5     
Flag Coverage Δ
#os_linux 77.39% <86.67%> (+0.14%) ⬆️
#os_macos 73.39% <86.67%> (+0.22%) ⬆️
#os_windows 73.27% <85.04%> (+0.23%) ⬆️
#unit 74.96% <85.04%> (+0.20%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@dwillist dwillist force-pushed the 253_inspect_builder_enhancement branch from e6f5f5f to 9d2409b Compare August 4, 2020 14:26
…nhancement

Signed-off-by: dwillist <dthornton@vmware.com>
@dwillist dwillist force-pushed the 253_inspect_builder_enhancement branch from 9d2409b to 71f7c48 Compare August 4, 2020 21:07
Signed-off-by: dwillist <dthornton@vmware.com>
Copy link
Member

@dfreilich dfreilich 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 really great, thanks for the PR! I have a few nits and comments, but overall this is 🏅

One general question - looking at this, it's interesting that a lot of the legwork for inspect_builder happens in internal/commands, which seems to buck the normal trend of having it be part of the public API. I wonder what people think of moving this functionality to the public inspect_builder.go file.

inspect_builder.go Outdated Show resolved Hide resolved
inspect_builder_test.go Outdated Show resolved Hide resolved
internal/commands/inspect_builder.go Show resolved Hide resolved
internal/commands/inspect_builder.go Outdated Show resolved Hide resolved
internal/commands/inspect_builder.go Outdated Show resolved Hide resolved
internal/commands/inspect_builder_test.go Outdated Show resolved Hide resolved
internal/commands/inspect_builder_test.go Outdated Show resolved Hide resolved
internal/commands/inspect_builder_test.go Outdated Show resolved Hide resolved
internal/dist/dist_test.go Outdated Show resolved Hide resolved
internal/dist/dist_test.go Outdated Show resolved Hide resolved
- update unit test names,
- refactor detectionOrderOutput into understandable functions
- remove un-needed homepage case

Signed-off-by: dwillist <dthornton@vmware.com>
@dwillist dwillist force-pushed the 253_inspect_builder_enhancement branch from 5a3f21d to 252e834 Compare August 5, 2020 20:02
Signed-off-by: dwillist <dthornton@vmware.com>
@dwillist dwillist force-pushed the 253_inspect_builder_enhancement branch from f685dd9 to d1776b9 Compare August 6, 2020 19:55
@jromero
Copy link
Member

jromero commented Aug 7, 2020

@dwillist sorry for the delay in reviewing this. Based on the output from the description of this issue I have two questions.

  1. I know we've discussed moving over to a tree structure. I think, visually, that would be easier to digest. Here's an example from maven of what I would envision. Is there a reason why we can't or shouldn't move over to a tree structure?
  2. (Doesn't matter if we make it a tree structure). The Group # appear to be incremental as you nest. I would think that they are only incremental for their "level". For example, I expected something like:
Detection Order:
  Group #1:
    simple/nested-level-1
      Group #1:
        simple/nested-level-2@nested--version
          Group #1:
            simple/layers@simple-layers-version
            read/env@read-env-version
          Group #2:
            simple/layers-2@simple-layers-version
            read/env-2@read-env-version
    read/env@read-env-version                      (optional)

- alter how groups are counted

Signed-off-by: dwillist <dthornton@vmware.com>
@dwillist dwillist force-pushed the 253_inspect_builder_enhancement branch from 4d8f7f2 to 26ce9c0 Compare August 10, 2020 02:59
@jromero jromero added this to the 0.13.0 milestone Aug 11, 2020
@jromero jromero added the type/enhancement Issue that requests a new feature or improvement. label Aug 11, 2020
@jromero jromero self-assigned this Aug 11, 2020
@jromero
Copy link
Member

jromero commented Aug 11, 2020

Thanks @dwillist for this! I'm sure some buildpack users (and authors) are going to be very happy with it. :D

I made a few tweaks to it in a separate PR #795. Feel free to review the changes but once that PR goes in this one will too.

@jromero jromero merged commit 60333ab into buildpacks:main Aug 11, 2020
@jromero jromero changed the title update inspect-builder output Improve inspect-builder output Aug 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement Issue that requests a new feature or improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show buildpack's order in inspect-builder output
3 participants