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

Allow pass helper to be built for macOS #169

Merged
merged 2 commits into from Aug 21, 2022

Conversation

ellsclytn
Copy link
Contributor

Pass is described as "The Standard Unix Password Manager", and so it can
easily run on more than just Linux. Namely, it's supported on macOS. The
CLI is identical to the Linux build, which means the Linux helper code
for Pass is fully applicable toward the macOS build - a couple of
renames being the only needed thing, purely for semantic correctness.

Resolves #146

@GordonTheTurtle
Copy link

Please sign your commits following these rules:
https://github.com/moby/moby/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "pass-macos-and-linux" git@github.com:ellsclytn/docker-credential-helpers.git somewhere
$ cd somewhere
$ git commit --amend -s --no-edit
$ git push -f

Amending updates the existing PR. You DO NOT need to open a new one.

pass/pass.go Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Aug 20, 2022

Codecov Report

Merging #169 (cc29c66) into master (a9d6be0) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #169   +/-   ##
=======================================
  Coverage   55.25%   55.25%           
=======================================
  Files           9        9           
  Lines         666      666           
=======================================
  Hits          368      368           
  Misses        255      255           
  Partials       43       43           
Impacted Files Coverage Δ
pass/pass.go 65.48% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@thaJeztah
Copy link
Member

Most of this PR was already merged as part of #187 - I did a quick rebase to see what's remaining in this PR 😅

@crazy-max
Copy link
Member

crazy-max commented Aug 20, 2022

Would need to also update build-darwin stage in Dockerfile

Edit: @thaJeztah You beat me to it 😅

@thaJeztah
Copy link
Member

Yup, just realised, and just pushed before you commented 😂

Dockerfile Outdated
@@ -71,6 +71,9 @@ RUN --mount=type=bind,target=. \
xx-go install std
xx-go build -ldflags "$(cat /tmp/.ldflags)" -o /out/docker-credential-osxkeychain-${TARGETARCH}${TARGETVARIANT} ./osxkeychain/cmd/main_darwin.go
xx-verify /out/docker-credential-osxkeychain-${TARGETARCH}${TARGETVARIANT}

xx-go build -ldflags "$(cat /tmp/.ldflags)" -o /out/docker-credential-pass-${TARGETARCH}${TARGETVARIANT} ./pass/cmd/main.go
xx-verify /out/docker-credential-pass-${TARGETARCH}${TARGETVARIANT}
Copy link
Member

Choose a reason for hiding this comment

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

should add also ${TARGETOS} now that is not only linux.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, good one; wasn't sure if we collected them all in the same spot.

Let me have a look (we probably still need to look if we want to enable this by default and/or if that could mess up release pipelines elsewhere)

Dockerfile Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@thaJeztah thaJeztah force-pushed the pass-macos-and-linux branch 2 times, most recently from c156609 to 68e116e Compare August 20, 2022 19:24
ellsclytn and others added 2 commits August 21, 2022 16:18
Pass is described as "The Standard Unix Password Manager", and so it can
easily run on more than just Linux. Namely, it's supported on macOS. The
CLI is identical to the Linux build, which means the Linux helper code
for Pass is fully applicable toward the macOS build - a couple of
renames being the only needed thing, purely for semantic correctness.

Signed-off-by: Ellis Clayton <ellis@ellis.codes>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
Copy link
Member

Rebased to get a fresh run of CI with the latest changes; still green, so let me get this one in 👍

@thaJeztah thaJeztah merged commit ebd9dc6 into docker:master Aug 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pass variant macos support
7 participants