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

Add Chocolatey delivery Github Action #718

Merged
merged 6 commits into from Jul 6, 2020

Conversation

dfreilich
Copy link
Member

@dfreilich dfreilich commented Jun 30, 2020

Signed-off-by: David Freilich dfreilich@vmware.com

Summary

This adds a Github Action pipeline which, upon a new release, will create and publish a Chocolatey release.

This can't be fully tested through act, unfortunately, because act doesn't yet support windows virtual environments, and the choco action (despite the promise otherwise) must be run on a windows virtual env - but I tested it on my fork here, and it worked.

Note: This will necessitate a maintainer adding a secret for the chocolatey API key (I referred to it as CHOC_KEY, but I'm happy to change that if so desired).

Documentation

Related

Resolves #703

@dfreilich dfreilich requested a review from a team as a code owner June 30, 2020 00:14
@dfreilich
Copy link
Member Author

This package is based off this article, which clarified exactly what's needed if you're going to embed the binary inside.

Note:

  • Despite what the docs say here, the package should be named pack and not pack.portable; portable is only relevant if a package is going to be offered in multiple ways, and we aren't planning ATM to have an installer method for chocolatey.

@codecov
Copy link

codecov bot commented Jun 30, 2020

Codecov Report

Merging #718 into release/0.12.0 will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff               @@
##           release/0.12.0     #718   +/-   ##
===============================================
  Coverage           73.82%   73.82%           
===============================================
  Files                  72       72           
  Lines                5000     5000           
===============================================
  Hits                 3691     3691           
  Misses                996      996           
  Partials              313      313           
Flag Coverage Δ
#os_linux 76.35% <ø> (ø)
#os_macos 72.13% <ø> (+0.05%) ⬆️
#os_windows 72.08% <ø> (ø)
#unit 73.82% <ø> (ø)

run: |
file=${{ env.CHOC_PATH }}/pack.nuspec
sed -i "s/{{PACK_VERSION}}/${{ env.PACK_VERSION }}/g" $file
sed -i -e '/{{README}}/r README.md' -e 's/{{README}}//' $file
Copy link
Member Author

Choose a reason for hiding this comment

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

I use sed, rather than the replace-tokens job, because GH actions has an open issue with multiline strings, that made the formatting of the README go all wonky. See here

<tags>pack cloud-native-buildpacks cncf</tags>
<summary>pack is a CLI for building apps using Cloud Native Buildpacks.</summary>
<description>{{README}}
</description>
Copy link
Member Author

Choose a reason for hiding this comment

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

I found that the sed wouldn't replace the {{README}} string normally if I had the description close on the same line; instead, it would insert on the next line, outside of the <description> tag entirely.

Copy link
Member

Choose a reason for hiding this comment

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

sed is more foreign to me than standard RegEx so I've avoided using it. This here just adds to my avoidance. :)

Copy link
Member Author

Choose a reason for hiding this comment

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

I totally agree. I just couldn't get it to work otherwise, so...

README.md Outdated
@@ -14,7 +14,7 @@

## Usage

<img src="resources/pack-build.gif" width="600px" />
<img src="https://github.com/buildpacks/pack/raw/main/resources/pack-build.gif" width="600px" />
Copy link
Member Author

Choose a reason for hiding this comment

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

I'd love to hear another suggestion if this isn't the best idea! I could also include the resource in the package, but that doesn't seem like the nicest solution, it would just needlessly clog the chocolatey package.

Copy link
Member

@jromero jromero Jun 30, 2020

Choose a reason for hiding this comment

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

TLDR; Might be worth NOT posting a README.md

I'm looking around at some other packages and how this file would be used and I have a few concerns:

  1. I haven't found that this README.md is used anywhere by another project (granted my search was quick).
  2. How would this README be presented? If it's not and it's only listed in the files then what's its purpose?
  3. The README.md is currently tailored to be hosted on GH. Some package registries (such as npm) do a good job about resolving links but chocolatey doesn't.

Based on the above and the added complexity/concerns, I would suggest that the <description> is sufficient from an end-user's perspective. I'm assuming it can then refer to (link) to additional details.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm pretty sure that this is how it gets displayed, which seems pretty visible to me: https://chocolatey.org/packages/buildpacks#description

Copy link
Member

Choose a reason for hiding this comment

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

Is there a way to confirm? If I download the nupkg. The README.md isn't there. Instead the content is in the <description> field:

image
image

Copy link
Member Author

Choose a reason for hiding this comment

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

To clarify what this change is about - above, we read in the README and putting it into the description tag. We don't publish a separate README file in the nupkg.

.github/workflows/delivery/chocolatey/pack.nuspec Outdated Show resolved Hide resolved
<tags>pack cloud-native-buildpacks cncf</tags>
<summary>pack is a CLI for building apps using Cloud Native Buildpacks.</summary>
<description>{{README}}
</description>
Copy link
Member

Choose a reason for hiding this comment

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

sed is more foreign to me than standard RegEx so I've avoided using it. This here just adds to my avoidance. :)

README.md Outdated
@@ -14,7 +14,7 @@

## Usage

<img src="resources/pack-build.gif" width="600px" />
<img src="https://github.com/buildpacks/pack/raw/main/resources/pack-build.gif" width="600px" />
Copy link
Member

@jromero jromero Jun 30, 2020

Choose a reason for hiding this comment

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

TLDR; Might be worth NOT posting a README.md

I'm looking around at some other packages and how this file would be used and I have a few concerns:

  1. I haven't found that this README.md is used anywhere by another project (granted my search was quick).
  2. How would this README be presented? If it's not and it's only listed in the files then what's its purpose?
  3. The README.md is currently tailored to be hosted on GH. Some package registries (such as npm) do a good job about resolving links but chocolatey doesn't.

Based on the above and the added complexity/concerns, I would suggest that the <description> is sufficient from an end-user's perspective. I'm assuming it can then refer to (link) to additional details.

@jromero jromero changed the base branch from main to release/0.12.0 July 1, 2020 21:34
@jromero
Copy link
Member

jromero commented Jul 1, 2020

NOTE: I changed the target branch for this PR in order (and hopes) that this will apply to the upcoming release. The workflows (including for the publish event) only trigger when included in the branch/commit that is tagged for release.

See: https://github.community/t/actions-not-triggered-on-release/17944/6

@jromero jromero added this to the 0.12.0 milestone Jul 6, 2020
@jromero jromero added type/chore Issue that requests non-user facing changes. roadmap/user-onboarding Help users (end-users, contributors, etc) get started with CNB. labels Jul 6, 2020
* Closes #703

Signed-off-by: David Freilich <dfreilich@vmware.com>
Signed-off-by: David Freilich <dfreilich@vmware.com>
* Used sed, rather than outputting the file from another job and inserting it using the tokens task, because Github Actions has an issue with multiline strings, and that would have removed formatting from it.

* Use remote link for usage gif so it won't break when added to
  chocolatey package readme.

Signed-off-by: David Freilich <dfreilich@vmware.com>
Signed-off-by: David Freilich <dfreilich@vmware.com>
* Remove changed link from README
* Stop putting Readme into nuspec

Signed-off-by: David Freilich <dfreilich@vmware.com>
Signed-off-by: David Freilich <dfreilich@vmware.com>
@jromero jromero merged commit 658c93a into release/0.12.0 Jul 6, 2020
@jromero jromero deleted the fix/703-publish-chocolatey branch July 6, 2020 17:18
@dfreilich
Copy link
Member Author

@jromero Just to confirm, was an API key added in under secrets for this line? https://github.com/buildpacks/pack/pull/718/files#diff-186c3d9e52fe0339dd81e7ce89864e4eR76

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
roadmap/user-onboarding Help users (end-users, contributors, etc) get started with CNB. type/chore Issue that requests non-user facing changes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Publish to Chocolatey
2 participants