user/dev discussion of public-inbox itself
 help / color / mirror / code / Atom feed
Search results ordered by [date|relevance]  view[summary|nested|Atom feed]
thread overview below | download mbox.gz: |
* [PATCH] import: fix handling of init.defaultBranch
  @ 2024-03-10 21:41  5%       ` Eric Wong
  0 siblings, 0 replies; 3+ results
From: Eric Wong @ 2024-03-10 21:41 UTC (permalink / raw)
  To: Rob Herring; +Cc: meta

Eric Wong <e@80x24.org> wrote:
> Will try to fix ASAP but offline problems persist :<

Fortunately it turned out to be a simple fix :x

-------8<-------
Subject: [PATCH] import: fix handling of init.defaultBranch

We must chomp the newline in the branch name if it's set.

Reported-by: Rob Herring <robh@kernel.org>
Link: https://public-inbox.org/meta/CAL_JsqK7P4gjLPyvzxNEcYmxT4j6Ah5f3Pz1RqDHxmysTg3aEg@mail.gmail.com/
Fixes: 73830410e4336b77 (treewide: use run_qx where appropriate, 2023-10-27)
---
 lib/PublicInbox/Import.pm | 1 +
 t/clone-coderepo.t        | 8 +++++---
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/lib/PublicInbox/Import.pm b/lib/PublicInbox/Import.pm
index a951874b..ed34d548 100644
--- a/lib/PublicInbox/Import.pm
+++ b/lib/PublicInbox/Import.pm
@@ -26,6 +26,7 @@ sub default_branch () {
 	state $default_branch = do {
 		my $h = run_qx([qw(git config --global init.defaultBranch)],
 				 { GIT_CONFIG => undef });
+		chomp $h;
 		$h eq '' ? 'refs/heads/master' : "refs/heads/$h";
 	}
 }
diff --git a/t/clone-coderepo.t b/t/clone-coderepo.t
index c0951941..c6180fc4 100644
--- a/t/clone-coderepo.t
+++ b/t/clone-coderepo.t
@@ -7,6 +7,7 @@ use PublicInbox::Import;
 use File::Temp;
 use File::Path qw(remove_tree);
 use PublicInbox::SHA qw(sha1_hex);
+use PublicInbox::IO;
 require_mods(qw(json Plack::Builder HTTP::Date HTTP::Status));
 require_git_http_backend;
 require_git '1.8.5';
@@ -38,9 +39,10 @@ my $t0 = time - 1;
 my $m; # manifest hashref
 
 {
-	my $rdr = {};
-	my $fi_data = './t/git.fast-import-data';
-	open $rdr->{0}, '<', $fi_data or xbail "open($fi_data): $!";
+	my $fi_data = PublicInbox::IO::try_cat './t/git.fast-import-data';
+	my $db = PublicInbox::Import::default_branch;
+	$fi_data =~ s!\brefs/heads/master\b!$db!gs;
+	my $rdr = { 0 => \$fi_data };
 	my @git = ('git', "--git-dir=$pa");
 	xsys_e([@git, qw(fast-import --quiet)], undef, $rdr);
 	xsys_e([qw(/bin/cp -Rp a.git b.git)], undef, { -C => "$tmpdir/src" });

^ permalink raw reply related	[relevance 5%]

* [PATCH 07/10] treewide: use run_qx where appropriate
  2023-10-27 22:21  7% [PATCH 00/10] first steps towards eliminating TIEHANDLE Eric Wong
@ 2023-10-27 22:21  3% ` Eric Wong
  0 siblings, 0 replies; 3+ results
From: Eric Wong @ 2023-10-27 22:21 UTC (permalink / raw)
  To: meta

This saves us some code, and is a small step towards getting
ProcessIO working with stat, fcntl and other perlops that don't
work with tied handles.
---
 lib/PublicInbox/Admin.pm           |  7 +++----
 lib/PublicInbox/Config.pm          | 14 ++++++--------
 lib/PublicInbox/Fetch.pm           |  7 +++----
 lib/PublicInbox/Import.pm          |  6 ++----
 lib/PublicInbox/LEI.pm             |  7 +++----
 lib/PublicInbox/LeiBlob.pm         | 12 +++++-------
 lib/PublicInbox/LeiMirror.pm       |  7 +++----
 lib/PublicInbox/MultiGit.pm        |  7 +++----
 lib/PublicInbox/Spamcheck/Spamc.pm | 11 +++--------
 lib/PublicInbox/XapHelperCxx.pm    | 11 +++--------
 t/solver_git.t                     | 12 ++++--------
 11 files changed, 38 insertions(+), 63 deletions(-)

diff --git a/lib/PublicInbox/Admin.pm b/lib/PublicInbox/Admin.pm
index 3140afad..893f4a1b 100644
--- a/lib/PublicInbox/Admin.pm
+++ b/lib/PublicInbox/Admin.pm
@@ -9,7 +9,7 @@ use parent qw(Exporter);
 our @EXPORT_OK = qw(setup_signals);
 use PublicInbox::Config;
 use PublicInbox::Inbox;
-use PublicInbox::Spawn qw(popen_rd);
+use PublicInbox::Spawn qw(run_qx);
 use PublicInbox::Eml;
 *rel2abs_collapsed = \&PublicInbox::Config::rel2abs_collapsed;
 
@@ -67,9 +67,8 @@ sub resolve_git_dir {
 	my ($cd) = @_;
 	# try v1 bare git dirs
 	my $cmd = [ qw(git rev-parse --git-dir) ];
-	my $fh = popen_rd($cmd, undef, {-C => $cd});
-	my $dir = do { local $/; <$fh> };
-	close $fh or die "error in @$cmd (cwd:${\($cd // '.')}): $?\n";
+	my $dir = run_qx($cmd, undef, {-C => $cd});
+	die "error in @$cmd (cwd:${\($cd // '.')}): $?\n" if $?;
 	chomp $dir;
 	# --absolute-git-dir requires git v2.13.0+
 	$dir = rel2abs_collapsed($dir, $cd) if $dir !~ m!\A/!;
diff --git a/lib/PublicInbox/Config.pm b/lib/PublicInbox/Config.pm
index d156b2d3..0a572103 100644
--- a/lib/PublicInbox/Config.pm
+++ b/lib/PublicInbox/Config.pm
@@ -13,7 +13,7 @@ use v5.10.1;
 use parent qw(Exporter);
 our @EXPORT_OK = qw(glob2re rel2abs_collapsed);
 use PublicInbox::Inbox;
-use PublicInbox::Spawn qw(popen_rd);
+use PublicInbox::Spawn qw(popen_rd run_qx);
 our $LD_PRELOAD = $ENV{LD_PRELOAD}; # only valid at startup
 our $DEDUPE; # set to {} to dedupe or clear cache
 
@@ -576,23 +576,21 @@ sub urlmatch {
 	my (%env, %opt);
 	my $cmd = $self->config_cmd(\%env, \%opt);
 	push @$cmd, @bool, qw(--includes -z --get-urlmatch), $key, $url;
-	my $fh = popen_rd($cmd, \%env, \%opt);
-	local $/ = "\0";
-	my $val = <$fh>;
-	if (!close($fh)) {
+	my $val = run_qx($cmd, \%env, \%opt);
+	if ($?) {
 		undef $val;
 		if (@bool && ($? >> 8) == 128) { # not boolean
 		} elsif (($? >> 8) != 1) {
 			$urlmatch_broken = 1;
 		} elsif ($try_git) { # n.b. this takes cwd into account
-			$fh = popen_rd([qw(git config), @bool,
+			$val = run_qx([qw(git config), @bool,
 					qw(-z --get-urlmatch), $key, $url]);
-			$val = <$fh>;
-			close($fh) or undef($val);
+			undef $val if $?;
 		}
 	}
 	$? = 0; # don't influence lei exit status
 	if (defined($val)) {
+		local $/ = "\0";
 		chomp $val;
 		$val = git_bool($val) if @bool;
 	}
diff --git a/lib/PublicInbox/Fetch.pm b/lib/PublicInbox/Fetch.pm
index e41dd448..b0f1437c 100644
--- a/lib/PublicInbox/Fetch.pm
+++ b/lib/PublicInbox/Fetch.pm
@@ -5,7 +5,7 @@ package PublicInbox::Fetch;
 use v5.12;
 use parent qw(PublicInbox::IPC);
 use URI ();
-use PublicInbox::Spawn qw(popen_rd run_wait);
+use PublicInbox::Spawn qw(popen_rd run_qx run_wait);
 use PublicInbox::Admin;
 use PublicInbox::LEI;
 use PublicInbox::LeiCurl;
@@ -20,9 +20,8 @@ sub remote_url ($$) {
 	my $rn = $lei->{opt}->{'try-remote'} // [ 'origin', '_grokmirror' ];
 	for my $r (@$rn) {
 		my $cmd = [ qw(git config), "remote.$r.url" ];
-		my $fh = popen_rd($cmd, undef, { -C => $dir, 2 => $lei->{2} });
-		my $url = <$fh>;
-		close $fh or next;
+		my $url = run_qx($cmd, undef, { -C => $dir, 2 => $lei->{2} });
+		next if $?;
 		$url =~ s!/*\n!!s;
 		return $url;
 	}
diff --git a/lib/PublicInbox/Import.pm b/lib/PublicInbox/Import.pm
index fb70b91b..6eee8774 100644
--- a/lib/PublicInbox/Import.pm
+++ b/lib/PublicInbox/Import.pm
@@ -8,7 +8,7 @@
 package PublicInbox::Import;
 use v5.12;
 use parent qw(PublicInbox::Lock);
-use PublicInbox::Spawn qw(run_die popen_rd spawn);
+use PublicInbox::Spawn qw(run_die run_qx spawn);
 use PublicInbox::MID qw(mids mid2path);
 use PublicInbox::Address;
 use PublicInbox::Smsg;
@@ -25,10 +25,8 @@ use PublicInbox::Git qw(read_all);
 
 sub default_branch () {
 	state $default_branch = do {
-		my $r = popen_rd([qw(git config --global init.defaultBranch)],
+		my $h = run_qx([qw(git config --global init.defaultBranch)],
 				 { GIT_CONFIG => undef });
-		chomp(my $h = <$r> // '');
-		CORE::close $r;
 		$h eq '' ? 'refs/heads/master' : "refs/heads/$h";
 	}
 }
diff --git a/lib/PublicInbox/LEI.pm b/lib/PublicInbox/LEI.pm
index e060bcbe..0f6f7f6f 100644
--- a/lib/PublicInbox/LEI.pm
+++ b/lib/PublicInbox/LEI.pm
@@ -19,7 +19,7 @@ use IO::Handle ();
 use Fcntl qw(SEEK_SET);
 use PublicInbox::Config;
 use PublicInbox::Syscall qw(EPOLLIN);
-use PublicInbox::Spawn qw(run_wait popen_rd);
+use PublicInbox::Spawn qw(run_wait popen_rd run_qx);
 use PublicInbox::Lock;
 use PublicInbox::Eml;
 use PublicInbox::Import;
@@ -1091,9 +1091,8 @@ sub path_to_fd {
 # caller needs to "-t $self->{1}" to check if tty
 sub start_pager {
 	my ($self, $new_env) = @_;
-	my $fh = popen_rd([qw(git var GIT_PAGER)]);
-	chomp(my $pager = <$fh> // '');
-	close($fh) or warn "`git var PAGER' error: \$?=$?";
+	chomp(my $pager = run_qx([qw(git var GIT_PAGER)]));
+	warn "`git var PAGER' error: \$?=$?" if $?;
 	return if $pager eq 'cat' || $pager eq '';
 	$new_env //= {};
 	$new_env->{LESS} //= 'FRX';
diff --git a/lib/PublicInbox/LeiBlob.pm b/lib/PublicInbox/LeiBlob.pm
index 40f64bd9..648d35b6 100644
--- a/lib/PublicInbox/LeiBlob.pm
+++ b/lib/PublicInbox/LeiBlob.pm
@@ -7,7 +7,7 @@ package PublicInbox::LeiBlob;
 use strict;
 use v5.10.1;
 use parent qw(PublicInbox::IPC);
-use PublicInbox::Spawn qw(run_wait popen_rd which);
+use PublicInbox::Spawn qw(run_wait run_qx which);
 use PublicInbox::DS;
 use PublicInbox::Eml;
 use PublicInbox::Git qw(read_all);
@@ -23,9 +23,8 @@ sub get_git_dir ($$) {
 	} else { # implicit --cwd, quiet errors
 		open $opt->{2}, '>', '/dev/null' or die "open /dev/null: $!";
 	}
-	my $r = popen_rd($cmd, {GIT_DIR => undef}, $opt);
-	chomp(my $gd = do { local $/; <$r> });
-	close($r) ? $gd : undef;
+	chomp(my $git_dir = run_qx($cmd, {GIT_DIR => undef}, $opt));
+	$? ? undef : $git_dir;
 }
 
 sub solver_user_cb { # called by solver when done
@@ -122,9 +121,8 @@ sub lei_blob {
 		my $cmd = [ 'git', '--git-dir='.$lei->ale->git->{git_dir},
 				'cat-file', 'blob', $blob ];
 		if (defined $lei->{-attach_idx}) {
-			my $fh = popen_rd($cmd, $lei->{env}, $rdr);
-			my $buf = do { local $/; <$fh> };
-			return extract_attach($lei, $blob, \$buf) if close($fh);
+			my $buf = run_qx($cmd, $lei->{env}, $rdr);
+			return extract_attach($lei, $blob, \$buf) unless $?;
 		}
 		$rdr->{1} = $lei->{1};
 		my $cerr = run_wait($cmd, $lei->{env}, $rdr) or return;
diff --git a/lib/PublicInbox/LeiMirror.pm b/lib/PublicInbox/LeiMirror.pm
index 43e59e6c..c2f7cbed 100644
--- a/lib/PublicInbox/LeiMirror.pm
+++ b/lib/PublicInbox/LeiMirror.pm
@@ -7,7 +7,7 @@ use v5.12;
 use parent qw(PublicInbox::IPC);
 use IO::Uncompress::Gunzip qw(gunzip $GunzipError);
 use IO::Compress::Gzip qw(gzip $GzipError);
-use PublicInbox::Spawn qw(popen_rd spawn run_wait run_die);
+use PublicInbox::Spawn qw(spawn run_wait run_die run_qx);
 use File::Path ();
 use File::Temp ();
 use File::Spec ();
@@ -57,9 +57,8 @@ sub try_scrape {
 	my $curl = $self->{curl} //= PublicInbox::LeiCurl->new($lei) or return;
 	my $cmd = $curl->for_uri($lei, $uri, '--compressed');
 	my $opt = { 0 => $lei->{0}, 2 => $lei->{2} };
-	my $fh = popen_rd($cmd, undef, $opt);
-	my $html = do { local $/; <$fh> } // die "read(curl $uri): $!";
-	CORE::close($fh) or return $lei->child_error($?, "@$cmd failed");
+	my $html = run_qx($cmd, undef, $opt);
+	return $lei->child_error($?, "@$cmd failed") if $?;
 
 	# we grep with URL below, we don't want Subject/From headers
 	# making us clone random URLs.  This assumes remote instances
diff --git a/lib/PublicInbox/MultiGit.pm b/lib/PublicInbox/MultiGit.pm
index 35bd0251..9074758a 100644
--- a/lib/PublicInbox/MultiGit.pm
+++ b/lib/PublicInbox/MultiGit.pm
@@ -5,7 +5,7 @@
 package PublicInbox::MultiGit;
 use strict;
 use v5.10.1;
-use PublicInbox::Spawn qw(run_die popen_rd);
+use PublicInbox::Spawn qw(run_die run_qx);
 use PublicInbox::Import;
 use File::Temp 0.19;
 use List::Util qw(max);
@@ -112,9 +112,8 @@ sub epoch_cfg_set {
 	my $f = epoch_dir($self)."/$epoch_nr.git/config";
 	my $v = "../../$self->{all}/config";
 	if (-r $f) {
-		my $rd = popen_rd([qw(git config -f), $f, 'include.path']);
-		chomp(my $have = <$rd> // '');
-		return if $have eq $v;
+		chomp(my $x = run_qx([qw(git config -f), $f, 'include.path']));
+		return if $x eq $v;
 	}
 	run_die([qw(git config -f), $f, 'include.path', $v ]);
 }
diff --git a/lib/PublicInbox/Spamcheck/Spamc.pm b/lib/PublicInbox/Spamcheck/Spamc.pm
index cba33a66..798de218 100644
--- a/lib/PublicInbox/Spamcheck/Spamc.pm
+++ b/lib/PublicInbox/Spamcheck/Spamc.pm
@@ -4,7 +4,7 @@
 # Default spam filter class for wrapping spamc(1)
 package PublicInbox::Spamcheck::Spamc;
 use v5.12;
-use PublicInbox::Spawn qw(popen_rd run_wait);
+use PublicInbox::Spawn qw(run_qx run_wait);
 use IO::Handle;
 use Fcntl qw(SEEK_SET);
 
@@ -20,14 +20,9 @@ sub new {
 sub spamcheck {
 	my ($self, $msg, $out) = @_;
 
+	$out = \(my $buf = '') unless ref($out);
 	my $rdr = { 0 => _msg_to_fh($self, $msg) };
-	my $fh = popen_rd($self->{checkcmd}, undef, $rdr);
-	unless (ref $out) {
-		my $buf = '';
-		$out = \$buf;
-	}
-	$$out = do { local $/; <$fh> };
-	close $fh; # PublicInbox::ProcessIO::CLOSE
+	$$out = run_qx($self->{checkcmd}, undef, $rdr);
 	($? || $$out eq '') ? 0 : 1;
 }
 
diff --git a/lib/PublicInbox/XapHelperCxx.pm b/lib/PublicInbox/XapHelperCxx.pm
index bd616b6f..83503035 100644
--- a/lib/PublicInbox/XapHelperCxx.pm
+++ b/lib/PublicInbox/XapHelperCxx.pm
@@ -7,7 +7,7 @@
 # The resulting executable is not linked to Perl in any way.
 package PublicInbox::XapHelperCxx;
 use v5.12;
-use PublicInbox::Spawn qw(popen_rd which);
+use PublicInbox::Spawn qw(run_qx which);
 use PublicInbox::Git qw(read_all);
 use PublicInbox::Search;
 use Fcntl qw(SEEK_SET);
@@ -29,14 +29,9 @@ my $xflags = ($ENV{CXXFLAGS} // '-Wall -ggdb3 -O0') . ' ' .
 my $xap_modversion;
 
 sub xap_cfg (@) {
-	use autodie qw(open seek);
-	open my $err, '+>', undef;
 	my $cmd = [ $ENV{PKG_CONFIG} // 'pkg-config', @_, 'xapian-core' ];
-	my $rd = popen_rd($cmd, undef, { 2 => $err });
-	chomp(my $ret = do { local $/; <$rd> });
-	return $ret if close($rd);
-	seek($err, 0, SEEK_SET);
-	$err = read_all($err);
+	chomp(my $ret = run_qx($cmd, undef, { 2 => \(my $err) }));
+	return $ret if !$?;
 	die <<EOM;
 @$cmd failed: Xapian development files missing? (\$?=$?)
 $err
diff --git a/t/solver_git.t b/t/solver_git.t
index 4f09e05b..ab8aba15 100644
--- a/t/solver_git.t
+++ b/t/solver_git.t
@@ -6,7 +6,7 @@ use PublicInbox::TestCommon;
 use Cwd qw(abs_path);
 require_git v2.6;
 use PublicInbox::ContentHash qw(git_sha);
-use PublicInbox::Spawn qw(popen_rd);
+use PublicInbox::Spawn qw(run_qx);
 require_mods(qw(DBD::SQLite Xapian URI::Escape));
 require PublicInbox::SolverGit;
 my $rdr = { 2 => \(my $null) };
@@ -234,13 +234,9 @@ SKIP: {
 		my $cmd = [ qw(git hash-object -w --stdin) ];
 		my $env = { GIT_DIR => $binfoo };
 		while (my ($label, $size) = each %bin) {
-			pipe(my ($rin, $win)) or BAIL_OUT;
-			my $rout = popen_rd($cmd , $env, { 0 => $rin });
-			$rin = undef;
-			print { $win } ("\0" x $size) or BAIL_OUT;
-			close $win or BAIL_OUT;
-			chomp(my $x = <$rout>);
-			close $rout or BAIL_OUT "$?";
+			my $rdr = { 0 => \("\0" x $size) };
+			chomp(my $x = run_qx($cmd , $env, $rdr));
+			xbail "@$cmd: \$?=$?" if $?;
 			$oid{$label} = $x;
 		}
 

^ permalink raw reply related	[relevance 3%]

* [PATCH 00/10] first steps towards eliminating TIEHANDLE
@ 2023-10-27 22:21  7% Eric Wong
  2023-10-27 22:21  3% ` [PATCH 07/10] treewide: use run_qx where appropriate Eric Wong
  0 siblings, 1 reply; 3+ results
From: Eric Wong @ 2023-10-27 22:21 UTC (permalink / raw)
  To: meta

ProcessIO being tied is problematic since perlops like stat
and fcntl don't work directly on the handle, and we need
separate classes to do buffered read and unbuffered sysread.

It's possible to do as common packages (e.g. File::Temp,
IO::Socket::IP, IO::Socket::SSL) do by subclassing IO::Handle
to avoid tie completely.  I also want to avoid the self-tie
thing IO::Socket::SSL to avoid having to rely on weaken().

So, this essentially becomes a war on `close($fh)' an relying
on $fh->close where appropriate.  This loses some usefulness
of autodie, so we keep `close($fh)' for places which aren't
tied at the moment.

One key place is to replace many calls to popen_rd with the new
run_qx to consolidate close calls.  There's also few other
cleanups, code reductions, and safety improvements along the way.

And more changes coming.

Eric Wong (10):
  spawn: croak directly in C pi_fork_exec
  spawnpp: use autodie for syscall failures
  spawn: avoid alloca in C pi_fork_exec
  git: use run_qx to read `git --version'
  git: avoid extra stat(2) for git version
  gcf2: simplify pkg-config and Inline::C setup
  treewide: use run_qx where appropriate
  www_altid: reduce FD pressure in qspawn queues
  xt/eml_check_limits: remove useless import
  lei_ale: use v5.12, autodie, and try_cat

 lib/PublicInbox/Admin.pm           |  7 ++---
 lib/PublicInbox/Config.pm          | 14 ++++-----
 lib/PublicInbox/Fetch.pm           |  7 ++---
 lib/PublicInbox/Gcf2.pm            | 27 +++++-----------
 lib/PublicInbox/Git.pm             | 15 +++++----
 lib/PublicInbox/Import.pm          |  6 ++--
 lib/PublicInbox/LEI.pm             |  7 ++---
 lib/PublicInbox/LeiALE.pm          | 24 +++++++--------
 lib/PublicInbox/LeiBlob.pm         | 12 +++-----
 lib/PublicInbox/LeiMirror.pm       |  7 ++---
 lib/PublicInbox/MultiGit.pm        |  7 ++---
 lib/PublicInbox/Spamcheck/Spamc.pm | 11 ++-----
 lib/PublicInbox/Spawn.pm           | 49 +++++++++++++++---------------
 lib/PublicInbox/SpawnPP.pm         | 10 +++---
 lib/PublicInbox/WwwAltId.pm        |  9 ++----
 lib/PublicInbox/XapHelperCxx.pm    | 11 ++-----
 t/solver_git.t                     | 12 +++-----
 xt/eml_check_limits.t              |  4 +--
 18 files changed, 94 insertions(+), 145 deletions(-)


^ permalink raw reply	[relevance 7%]

Results 1-3 of 3 | reverse | options above
-- pct% links below jump to the message on this page, permalinks otherwise --
2023-10-27 22:21  7% [PATCH 00/10] first steps towards eliminating TIEHANDLE Eric Wong
2023-10-27 22:21  3% ` [PATCH 07/10] treewide: use run_qx where appropriate Eric Wong
2024-03-06 15:44     crash on git-fast-import Rob Herring
2024-03-08  3:20     ` Eric Wong
2024-03-08 14:38       ` Rob Herring
2024-03-10 21:15         ` Eric Wong
2024-03-10 21:41  5%       ` [PATCH] import: fix handling of init.defaultBranch Eric Wong

Code repositories for project(s) associated with this public inbox

	https://80x24.org/public-inbox.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).