threads / patch / 52468

patch, 2 partscontrib/credential/netrc: Makefile + script cleanup

Subject: [PATCH 0/2] contrib/credential/netrc: Makefile + script cleanup

## tl;dr

9 messages between Dec 16, 2019 and Dec 20, 2019. Diffs are folded; open one to read it.

replies: 8people: 2as markdown or json

Denton Liu· Dec 16, 2019, 21:32 UTC · lore

I recently switched my workflow to use this credential helper and I noticed a couple of problems:

1. The interpreter path was hardcoded to #!/usr/bin/perl
2. The script refuses to run outside of a Git repository
This patch series should fix these problems.
Denton Liu (2):
  contrib/credential/netrc: make PERL_PATH configurable
  contrib/credential/netrc: work outside a repo
 contrib/credential/netrc/.gitignore           |  1 +
 contrib/credential/netrc/Makefile             | 26 +++++++++++++++++--
 ...ential-netrc => git-credential-netrc.perl} |  2 +-
 3 files changed, 26 insertions(+), 3 deletions(-)
 create mode 100644 contrib/credential/netrc/.gitignore
 rename contrib/credential/netrc/{git-credential-netrc => git-credential-netrc.perl} (99%)
-- 
2.24.1.664.g198078bb5a
Denton Liu· Dec 16, 2019, 21:32 UTC · re: Denton Liu · lore

[PATCH 2/2] contrib/credential/netrc: work outside a repo

Currently, git-credential-netrc does not work outside of a git repository. It fails with the following error:

	fatal: Not a git repository: . at /usr/share/perl5/Git.pm line 214.

There is no real reason why need to be within a repository, though. Credential helpers should be able to work just fine outside the repository as well.

Call the non-self version of config() so that git-credential-netrc no longer needs to be run within a repository.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 contrib/credential/netrc/git-credential-netrc.perl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to contrib/credential/netrc/git-credential-netrc.perl +1 −1
diff --git a/contrib/credential/netrc/git-credential-netrc.perl b/contrib/credential/netrc/git-credential-netrc.perl
index ebfc123ec6..bc57cc6588 100755
--- a/contrib/credential/netrc/git-credential-netrc.perl
+++ b/contrib/credential/netrc/git-credential-netrc.perl
@@ -423,7 +423,7 @@ sub load_config {
 	# load settings from git config
 	my $options = shift;
 	# set from command argument, gpg.program option, or default to gpg
-	$options->{'gpg'} //= Git->repository()->config('gpg.program')
+	$options->{'gpg'} //= Git::config('gpg.program')
 	                  // 'gpg';
 	log_verbose("using $options{'gpg'} for GPG operations");
 }
-- 
2.24.1.664.g198078bb5a
Denton Liu· Dec 16, 2019, 21:32 UTC · re: Denton Liu · lore

[PATCH 1/2] contrib/credential/netrc: make PERL_PATH configurable

The shebang path for the Perl interpreter in git-credential-netrc was hardcoded. However, some users may have it located at a different location and thus, would have had to manually edit the script.

Add a .perl prefix to the script to denote it as a template and ignore the generated version. Augment the Makefile so that it generates git-credential-netrc from git-credential-netrc.perl, just like other Perl scripts.

The Makefile recipes were shamelessly stolen from contrib/mw-to-git/Makefile.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 contrib/credential/netrc/.gitignore           |  1 +
 contrib/credential/netrc/Makefile             | 26 +++++++++++++++++--
 ...ential-netrc => git-credential-netrc.perl} |  0
 3 files changed, 25 insertions(+), 2 deletions(-)
 create mode 100644 contrib/credential/netrc/.gitignore
 rename contrib/credential/netrc/{git-credential-netrc => git-credential-netrc.perl} (100%)
Show changes to 3 files +25 −2

contrib/credential/netrc/.gitignore, contrib/credential/netrc/Makefile, contrib/credential/netrc/git-credential-netrc

diff --git a/contrib/credential/netrc/.gitignore b/contrib/credential/netrc/.gitignore
new file mode 100644
index 0000000000..d41cdde84b
--- /dev/null
+++ b/contrib/credential/netrc/.gitignore
@@ -0,0 +1 @@
+git-credential-netrc
diff --git a/contrib/credential/netrc/Makefile b/contrib/credential/netrc/Makefile
index 6174e3bb83..c284fb8ac4 100644
--- a/contrib/credential/netrc/Makefile
+++ b/contrib/credential/netrc/Makefile
@@ -1,8 +1,30 @@
 # The default target of this Makefile is...
 all::
 
-test:
+SCRIPT_PERL = git-credential-netrc.perl
+GIT_ROOT_DIR = ../../..
+HERE = contrib/credential/netrc
+
+SCRIPT_PERL_FULL = $(patsubst %,$(HERE)/%,$(SCRIPT_PERL))
+
+all:: build
+
+build:
+	$(MAKE) -C $(GIT_ROOT_DIR) SCRIPT_PERL="$(SCRIPT_PERL_FULL)" \
+                build-perl-script
+
+install: build
+	$(MAKE) -C $(GIT_ROOT_DIR) SCRIPT_PERL="$(SCRIPT_PERL_FULL)" \
+                install-perl-script
+
+clean:
+	$(MAKE) -C $(GIT_ROOT_DIR) SCRIPT_PERL="$(SCRIPT_PERL_FULL)" \
+                clean-perl-script
+
+test: build
 	./t-git-credential-netrc.sh
 
-testverbose:
+testverbose: build
 	./t-git-credential-netrc.sh -d -v
+
+.PHONY: all build install clean test testverbose
diff --git a/contrib/credential/netrc/git-credential-netrc b/contrib/credential/netrc/git-credential-netrc.perl
similarity index 100%
rename from contrib/credential/netrc/git-credential-netrc
rename to contrib/credential/netrc/git-credential-netrc.perl
-- 
2.24.1.664.g198078bb5a
Jeff King· Dec 17, 2019, 02:48 UTC · re: Denton Liu · lore

Re: [PATCH 0/2] contrib/credential/netrc: Makefile + script cleanup

On Mon, Dec 16, 2019 at 01:32:32PM -0800, Denton Liu wrote:
Show 8 quoted lines
> I recently switched my workflow to use this credential helper and I
> noticed a couple of problems:
> 
> 1. The interpreter path was hardcoded to #!/usr/bin/perl
> 
> 2. The script refuses to run outside of a Git repository
> 
> This patch series should fix these problems.
Both of these patches look good to me.

It does make me wonder if many people are using credential-netrc, given these pretty obvious problems. You're certainly welcome to use it if it works for you, but I am curious what made you pick it versus one of the other more advanced helpers.

I assume you're using a gpg-encrypted netrc (if not, you should probably just use credential-store). For "read-only" password access, I find the combination of pass[1] with config like this is a bit nicer:

  [credential "https://github.com"]
  username = peff
  helper = "!f() { test $1 = get && echo password=`pass github/oauth`; }; f"
-Peff
[1] https://www.passwordstore.org/
Denton Liu· Dec 17, 2019, 06:17 UTC · re: Jeff King · lore

Re: [PATCH 0/2] contrib/credential/netrc: Makefile + script cleanup

Hi Peff,
On Mon, Dec 16, 2019 at 09:48:58PM -0500, Jeff King wrote:
Show 12 quoted lines
> On Mon, Dec 16, 2019 at 01:32:32PM -0800, Denton Liu wrote:
> 
> > I recently switched my workflow to use this credential helper and I
> > noticed a couple of problems:
> > 
> > 1. The interpreter path was hardcoded to #!/usr/bin/perl
> > 
> > 2. The script refuses to run outside of a Git repository
> > 
> > This patch series should fix these problems.
> 
> Both of these patches look good to me.
Thanks for reviewing :)
Show 5 quoted lines
> 
> It does make me wonder if many people are using credential-netrc, given
> these pretty obvious problems. You're certainly welcome to use it if it
> works for you, but I am curious what made you pick it versus one of the
> other more advanced helpers.

I wanted a credential helper that was encrypted and OS-independent. Since GPG is basically available on everything, it fits my use case.

It also helped that it resulted in a workflow that was very similar to my old mutt workflow. Before, I had

	source "gpg -dq ~/.mutt/credentials.gpg |"
and I just replaced the gpg with a script that wraps around git-credential.
> 
> I assume you're using a gpg-encrypted netrc (if not, you should probably
> just use credential-store). For "read-only" password access, I find the
> combination of pass[1] with config like this is a bit nicer:

Nice, I never knew heard about pass until now. I only have one password in my credential store currently but if I need to add more, I'll keep this in mind!

-Denton
Show 8 quoted lines
> 
>   [credential "https://github.com"]
>   username = peff
>   helper = "!f() { test $1 = get && echo password=`pass github/oauth`; }; f"
> 
> -Peff
> 
> [1] https://www.passwordstore.org/
Denton Liu· Dec 20, 2019, 18:44 UTC · re: Denton Liu · lore

[RESEND PATCH 0/2] contrib/credential/netrc: Makefile + script cleanup

I recently switched my workflow to use this credential helper and I noticed a couple of problems:

1. The interpreter path was hardcoded to #!/usr/bin/perl
2. The script refuses to run outside of a Git repository
This patch series should fix these problems.
Denton Liu (2):
  contrib/credential/netrc: make PERL_PATH configurable
  contrib/credential/netrc: work outside a repo
 contrib/credential/netrc/.gitignore           |  1 +
 contrib/credential/netrc/Makefile             | 26 +++++++++++++++++--
 ...ential-netrc => git-credential-netrc.perl} |  2 +-
 3 files changed, 26 insertions(+), 3 deletions(-)
 create mode 100644 contrib/credential/netrc/.gitignore
 rename contrib/credential/netrc/{git-credential-netrc => git-credential-netrc.perl} (99%)
-- 
2.24.1.703.g2f499f1283
Denton Liu· Dec 20, 2019, 18:45 UTC · re: Denton Liu · lore

[RESEND PATCH 1/2] contrib/credential/netrc: make PERL_PATH configurable

The shebang path for the Perl interpreter in git-credential-netrc was hardcoded. However, some users may have it located at a different location and thus, would have had to manually edit the script.

Add a .perl prefix to the script to denote it as a template and ignore the generated version. Augment the Makefile so that it generates git-credential-netrc from git-credential-netrc.perl, just like other Perl scripts.

The Makefile recipes were shamelessly stolen from contrib/mw-to-git/Makefile.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 contrib/credential/netrc/.gitignore           |  1 +
 contrib/credential/netrc/Makefile             | 26 +++++++++++++++++--
 ...ential-netrc => git-credential-netrc.perl} |  0
 3 files changed, 25 insertions(+), 2 deletions(-)
 create mode 100644 contrib/credential/netrc/.gitignore
 rename contrib/credential/netrc/{git-credential-netrc => git-credential-netrc.perl} (100%)
Show changes to 3 files +25 −2

contrib/credential/netrc/.gitignore, contrib/credential/netrc/Makefile, contrib/credential/netrc/git-credential-netrc

diff --git a/contrib/credential/netrc/.gitignore b/contrib/credential/netrc/.gitignore
new file mode 100644
index 0000000000..d41cdde84b
--- /dev/null
+++ b/contrib/credential/netrc/.gitignore
@@ -0,0 +1 @@
+git-credential-netrc
diff --git a/contrib/credential/netrc/Makefile b/contrib/credential/netrc/Makefile
index 6174e3bb83..c284fb8ac4 100644
--- a/contrib/credential/netrc/Makefile
+++ b/contrib/credential/netrc/Makefile
@@ -1,8 +1,30 @@
 # The default target of this Makefile is...
 all::
 
-test:
+SCRIPT_PERL = git-credential-netrc.perl
+GIT_ROOT_DIR = ../../..
+HERE = contrib/credential/netrc
+
+SCRIPT_PERL_FULL = $(patsubst %,$(HERE)/%,$(SCRIPT_PERL))
+
+all:: build
+
+build:
+	$(MAKE) -C $(GIT_ROOT_DIR) SCRIPT_PERL="$(SCRIPT_PERL_FULL)" \
+                build-perl-script
+
+install: build
+	$(MAKE) -C $(GIT_ROOT_DIR) SCRIPT_PERL="$(SCRIPT_PERL_FULL)" \
+                install-perl-script
+
+clean:
+	$(MAKE) -C $(GIT_ROOT_DIR) SCRIPT_PERL="$(SCRIPT_PERL_FULL)" \
+                clean-perl-script
+
+test: build
 	./t-git-credential-netrc.sh
 
-testverbose:
+testverbose: build
 	./t-git-credential-netrc.sh -d -v
+
+.PHONY: all build install clean test testverbose
diff --git a/contrib/credential/netrc/git-credential-netrc b/contrib/credential/netrc/git-credential-netrc.perl
similarity index 100%
rename from contrib/credential/netrc/git-credential-netrc
rename to contrib/credential/netrc/git-credential-netrc.perl
-- 
2.24.1.703.g2f499f1283
Denton Liu· Dec 20, 2019, 18:45 UTC · re: Denton Liu · lore

[RESEND PATCH 2/2] contrib/credential/netrc: work outside a repo

Currently, git-credential-netrc does not work outside of a git repository. It fails with the following error:

	fatal: Not a git repository: . at /usr/share/perl5/Git.pm line 214.

There is no real reason why need to be within a repository, though. Credential helpers should be able to work just fine outside the repository as well.

Call the non-self version of config() so that git-credential-netrc no longer needs to be run within a repository.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 contrib/credential/netrc/git-credential-netrc.perl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to contrib/credential/netrc/git-credential-netrc.perl +1 −1
diff --git a/contrib/credential/netrc/git-credential-netrc.perl b/contrib/credential/netrc/git-credential-netrc.perl
index ebfc123ec6..bc57cc6588 100755
--- a/contrib/credential/netrc/git-credential-netrc.perl
+++ b/contrib/credential/netrc/git-credential-netrc.perl
@@ -423,7 +423,7 @@ sub load_config {
 	# load settings from git config
 	my $options = shift;
 	# set from command argument, gpg.program option, or default to gpg
-	$options->{'gpg'} //= Git->repository()->config('gpg.program')
+	$options->{'gpg'} //= Git::config('gpg.program')
 	                  // 'gpg';
 	log_verbose("using $options{'gpg'} for GPG operations");
 }
-- 
2.24.1.703.g2f499f1283
Jeff King· Dec 20, 2019, 20:59 UTC · re: Denton Liu · lore

Re: [RESEND PATCH 0/2] contrib/credential/netrc: Makefile + script cleanup

On Fri, Dec 20, 2019 at 10:44:59AM -0800, Denton Liu wrote:
Show 8 quoted lines
> I recently switched my workflow to use this credential helper and I
> noticed a couple of problems:
> 
> 1. The interpreter path was hardcoded to #!/usr/bin/perl
> 
> 2. The script refuses to run outside of a Git repository
> 
> This patch series should fix these problems.
Since this is just a re-send, I'll re-send my:
  Reviewed-by: Jeff King <peff@peff.net>
:)
-Peff

← back to recent threads