Hello Daniel,
Dear all,
thanks for your detailed review, Daniel. Finally got some more leisure
time to care about it.
I deceided to concentrate on the first part to handle property-modified
files plus your overall comments.
Left the Windows support aside as this has to be fine-tuned a lot, as I
learned a lot from your comments, thanks.
On 2020-09-12 05:14, Daniel Shahaf wrote:
Nathan Hartman wrote on Fri, Sep 11, 2020 at 10:38:24 -0400:
Subject: [PATCH] Check also property-modified files in hook script
check-mime-type.pl
* contrib/hook-scripts/check-mime-type.pl:
(proplist) add property-modified files for checks
There are several changes not mentioned in the log message:
- Script name fix in comments
- Move 'use' before 'BEGIN'
(incidentally, I'm not actually sure that's a correct change, given
the BEGIN block implies perls as old as 5.4 are supported.)
- Add docstrings
+++ b/contrib/hook-scripts/check-mime-type.pl
@@ -1,18 +1,19 @@
#!/usr/bin/env perl
#
====================================================================
-# commit-mime-type-check.pl: check that every added file has the
-# svn:mime-type property set and every added file with a mime-type
-# matching text/* also has svn:eol-style set.
+# check-mime-type.pl: check that every added or property-modified
file
+# has the svn:mime-type property set and every added or
property-modified
+# file with a mime-type matching text/* also has svn:eol-style set.
I don't think it's acceptable for a script called "check-mime-type.pl"
to start enforcing svn:eol-style as well. This violates the principles
of least surprise and of "Do one thing and do it well". Also, the
choice of text/* is a bit arbitrary, and would seem to have both false
positives and false negatives: e.g., application/xml and text/x-diff.
I'm not even sure it's acceptable for this script to start enforcing
svn:mime-type's presence upon propmods, either. It's not exactly
a natural extension that existing users would expect.
First I am not the original author of that hook script, just using it
with those modifications with Subversion 1.6 up to 1.14.0 on
Debian/Ubuntu and Windows.
Some people (like me) want to enforce "svn:mime-type" for some
repositories and using that script is optional to admins.
I agree with the original author to check mime-type plus EoL in a single
script, as EoL setting can be important to text files.
Enforcing a policy on creation but not on changes is inconsistent,
that's the reason for this fix.
But I do understand your worries and made the functionalities optional,
setting the default exactly as it acts right now.
Completely removing the EoL check would change its fucntionality after
16 years.
New version of the patch, log and script itself attached. Will change
the Pull Request according to upcoming comments.
Additionally it could be renamed, e.g. to
"check-mime-type-and-text-eol.pl"?
#
====================================================================
# Copyright (c) 2000-2004 CollabNet. All rights reserved.
+# Copyright (c) 2014-2020 Apache Software Foundation (ASF).
How did you come up with these numbers? The script received commits in
2013 and 2015, so the end point should be 2013. (Also, the next line
talks of a "COPYING" file which doesn't exist, but that's a preëxisting
problem.)
Those lines are inherited from commit-access-control.pl, when it was
copied as a base for check-mime-type.pl.
Copyright lines and license were not updated by CollabNet or ASF despite
new commits since 2004.
I used 2014 as a start as this was the time this patch was posted on the
ASF mailing list.
Maybe coyprigght and license should be adapted to the current version of
commit-access-control.pl either with or without copyright lines.
According to the overall Subversion history it may should be changed to:
# Copyright (c) 2000-2009 CollabNet. All rights reserved.
# Copyright (c) 2010-2020 Apache Software Foundation (ASF).
@@ -100,19 +101,20 @@ my $tmp_dir = '/tmp';
-# Figure out what files have added using svnlook.
-my @files_added;
+# Figure out what files have been added/property-modified using
svnlook.
+my @paths_to_check;
+my $props_changed_re = qr/^(?:A |[U_ ]U) (.*[^\/])$/;
foreach my $line (&read_from_process($svnlook, 'changed', $repos,
'-t', $txn))
{
- # Add only files that were added to @files_added
- if ($line =~ /^A. (.*[^\/])$/)
+ # Add only files that were added/property-modified to
@paths_to_check
+ if ($line =~ /$props_changed_re/)
Why did you change /^A./ to /^A /?
The RegEx was taken from Leo Davis' mail, but I agree it should be /^A./
for added files and also /^.U/ for property-modified files.
Subject: [PATCH] Enhanced platform support of hook script
check-mime-type.pl
* contrib/hook-scripts/check-mime-type.pl:
(svnlook) Cross Platform: sane executable determination
+++ b/contrib/hook-scripts/check-mime-type.pl
@@ -28,6 +28,15 @@
use strict;
use Carp;
+use Config;
+use Cwd;
«use Cwd qw(getcwd);» would be better.
+use Env;
Why?
This syntax (according to «perldoc Env») "ties all existing environment
variables ("keys %ENV") to scalars". This seems like a dangerous thing
to do; in fact, it negates the «use strict» a few lines above.
I suspect this line should be deleted.
+use File::Which qw(which);
+use File::Temp qw(tempdir tempfile);
+
+my $old_dir;
+my $os_windows = $^O eq 'MSWin32';
+my $os_dos = $^O eq 'dos';
# Turn on warnings the best way depending on the Perl version.
BEGIN {
@@ -35,6 +44,12 @@ BEGIN {
{ require warnings; import warnings; }
else
{ $^W = 1; }
+
+ $old_dir = getcwd;
+}
+
+END {
+ chdir($old_dir);
}
Why is this necessary?
On Windows a sub script called by the precommit hook can change the
current directory whcih also affects the precommit hook, which had led
to weird results for the following sub scripts by the precommit hook.
This should have been explained in a comment.
@@ -42,31 +57,58 @@ BEGIN {
# Configuration section.
# Svnlook path.
-my $svnlook = "/usr/bin/svnlook";
+my $svnlook;
# Since the path to svnlook depends upon the local installation
# preferences, check that the required program exists to insure that
# the administrator has set up the script properly.
{
- my $ok = 1;
- foreach my $program ($svnlook)
+ my $svnlook_exe = 'svnlook' . $Config{_exe};
+ my @svnlook_paths;
+
+ if ($os_windows || $os_dos)
+ {
+ # On Windows/DOS the environment is available
+ my $svn_bindir = $ENV{'SVN_BINDIR'};
Shouldn't the condition be «if (defined $ENV{SVN_BINDIR})»?
+ if (defined $svn_bindir and length $svn_bindir)
+ {
+ $svnlook_paths[++$#svnlook_paths] =
"$svn_bindir/$svnlook_exe";
Why does this line use «$arrayvar[++$#arrayvar] = $foo» rather than
«push @arrayvar, $foo», which is equivalent and idiomatic?
Just because I'm not a Perl expert.
That's why I postponed a patch for Windows support.
+ }
+
+ $svnlook = which($svnlook_exe);
+ if (defined $svnlook and length $svnlook)
+ {
+ $svnlook_paths[++$#svnlook_paths] = $svnlook;
+ }
Why do we bother to have $svnlook tested for existence and
executability? Don't we trust File::Which::which() to work as
advertised?
+ undef $svnlook;
+ }
+
+ # On Unix the environment is empty, define fallback with absolute
path
+ $svnlook_paths[++$#svnlook_paths] = "/usr/bin/$svnlook_exe";
+ foreach my $program (@svnlook_paths)
{
if (-e $program)
{
unless (-x $program)
{
- warn "$0: required program `$program' is not
executable, ",
- "edit $0.\n";
- $ok = 0;
+ warn "$0: program `$program' is not executable.\n";
+ next;
}
+ $svnlook = $program;
+ last;
}
else
{
- warn "$0: required program `$program' does not exist, edit
$0.\n";
- $ok = 0;
+ warn "$0: program `$program' does not exist.\n";
+ next;
}
}
What's the point of issuing a warning when there are some more
iterations yet to go?
If I'm reading this correctly, this entire block should be reduced to:
# Use «||» rather than «//» since the BEGIN block implies versions
of Perl that lack «//» are supported.
my $svnlook = +(defined($ENV{SVN_BINDIR}) ?
"$ENV{SVN_BINDIR}/$svnlook_exe" : undef) || which("svnlook") ||
"/usr/bin/svnlook";
die … unless -x $svnlook and (-f _ or -l _);
I changed -e to (-f or -l) since -x implies -e.
I also disabled the fallbacks when the envvar is set but doesn't point
to a usable executable. I'm not married to these semantics, but I
think
these semantics may be expected _given the particular variable name_:
SVN_BINDIR is the name of the configure AC_SUBST() variable that's
cooked into various scripts installed by install-tools.
- exit 1 unless $ok;
+ unless (defined $svnlook and length $svnlook)
+ {
+ warn "$0: required program `$svnlook_exe' does not exist or is
not executable, maybe have to edit $0.\n";
+ exit 1;
+ }
}
######################################################################
@@ -95,9 +137,9 @@ sub ACCESS_READ_WRITE () { 'read-write' }
######################################################################
# Harvest data using svnlook.
-# Change into /tmp so that svnlook diff can create its .svnlook
+# Change into temp dir so that svnlook diff can create its .svnlook
When this script was first added, svnlook would create
a $TMPDIR/svnlook.$unique_integer directory (without a leading dot, and
in $TMPDIR rather than in cwd). svnlook doesn't do that nowadays, so
I see no reason to chdir at all — …
# directory.
-my $tmp_dir = '/tmp';
+my $tmp_dir = tempdir( TMPDIR => 1, CLEANUP => 1 );
… in which case, this line — though otherwise correct — should simply
be
deleted altogether, and the File::Temp and Cwd uses with it.
@@ -220,19 +262,66 @@ sub safe_read_from_pipe
+ while ($command = shift)
+ {
+ if ("-m" eq $command)
- This is a general-purpose "run a child process" function, so it can't
special-case "-m".
- If it could, it would have to check for all four variants: «-m foo»,
«-mfoo», «--message=foo», and «--message foo».
- It would also have to look for the «--» end-of-options guard, since
«-m» may occur after one. (This can actually happen in the second
callsite of read_from_process().)
- None of the callers pass -m in the first place, so this is dead code.
+ {
+ $comment = shift;
+ my ($handle, $tmpfile) = tempfile( DIR => $tmp_dir);
+ print $handle $comment;
+ close($handle);
+
+ push(@commandline, "--file");
+ push(@commandline, $tmpfile);
There may have been a "--" end-of-options guard earlier on the command
line.
+ }
+ else
+ {
+ # Munge the command to protect it from the command line
+ $command =~ s/\"/\\\"/g;
+ if ($command =~ m"\s") { $command = "\"$command\""; }
+ if ($command eq "") { $command = "\"\""; }
I'm pretty sure this escaping is incomplete. Use a library function
rather than roll your own escaping.
+ if ($command =~ m"\n")
+ {
+ warn "$0: carriage return detected in command - may
not work\n";
If it "may not work", then die() rather than warn().
+ }
+ push(@commandline, $command);
⋮
+ open(SAFE_READ, "@commandline |")
They can be applied in sequence:
svn-patch-01-c73cb4415d.txt
svn-patch-02-68cc555740.txt
In principle I think the changes are useful, though might change
behavior in a surprising way for those who are using the hook as it
stands now.
Usefulness aside, I think the first patch is unacceptable because of
backwards compatibility considerations and the second patch would not
be
acceptable unless revised.
Would it make sense to copy check-mime-type.pl, calling it, say,
check-mime-type-2.pl, and apply the changes to that file, keeping the
original as it is now?
For the first patch, moving the new behaviours to a new script would
address the backwards compatibility point, as would making the new
behaviours optional and opt-in. However, neither would address the
other review points.
The second patch doesn't appear to raise any backwards compatibility
concerns.
Cheers,
Daniel
Kind regards
Maddes
[[[
Enhance hook script check-mime-type.pl to also check property-modified files.
* contrib/hook-scripts/check-mime-type.pl:
-- add option to recognize property-modified files for checks
-- make check for svn:eol-style optional
-- enhance texts and comments to reflect changes
-- rename array "files_added" to "files_to_check" to better fit its usage
-- explicitly mark here document with quotes as interpolating
-- fix script name in comments
-- update copyright and license to reflect switch to ASF
Patch by: Matthias Bücher maddes+subvers...@maddes.net
Suggested by: Leo Davis [1]
[1]
https://mail-archives.apache.org/mod_mbox/subversion-dev/201403.mbox/%3c1576503.m6xb7ud...@hurry.speechfxinc.com%3E
]]]
diff --git a/contrib/hook-scripts/check-mime-type.pl b/contrib/hook-scripts/check-mime-type.pl
index 3ded119f27..867e5e5ce4 100755
--- a/contrib/hook-scripts/check-mime-type.pl
+++ b/contrib/hook-scripts/check-mime-type.pl
@@ -1,28 +1,36 @@
#!/usr/bin/env perl
# ====================================================================
-# commit-mime-type-check.pl: check that every added file has the
-# svn:mime-type property set and every added file with a mime-type
-# matching text/* also has svn:eol-style set. If any file fails this
-# test the user is sent a verbose error message suggesting solutions and
-# the commit is aborted.
+# check-mime-type.pl: check that every added or property-modified file
+# has the svn:mime-type property set and every added or property-modified
+# file with a mime-type matching text/* also has svn:eol-style set.
+# If any file fails this test the user is sent a verbose error message
+# suggesting solutions and the commit is aborted.
#
-# Usage: commit-mime-type-check.pl REPOS TXN-NAME
+# Usage: check-mime-type.pl REPOS TXN-NAME
# ====================================================================
-# Most of commit-mime-type-check.pl was taken from
+# Most of check-mime-type.pl was taken from
# commit-access-control.pl, Revision 9986, 2004-06-14 16:29:22 -0400.
# ====================================================================
-# Copyright (c) 2000-2004 CollabNet. All rights reserved.
+# Copyright (c) 2000-2009 CollabNet. All rights reserved.
+# Copyright (c) 2010-2020 Apache Software Foundation (ASF).
+# ====================================================================
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
#
-# This software is licensed as described in the file COPYING, which
-# you should have received as part of this distribution. The terms
-# are also available at http://subversion.tigris.org/license.html.
-# If newer versions of this license are posted there, you may use a
-# newer version instead, at your option.
+# http://www.apache.org/licenses/LICENSE-2.0
#
-# This software consists of voluntary contributions made by many
-# individuals. For exact contribution history, see the revision
-# history and logs, available at http://subversion.tigris.org/.
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
# ====================================================================
# Turn on warnings the best way depending on the Perl version.
@@ -40,6 +48,12 @@ use Carp;
######################################################################
# Configuration section.
+# Toggle: Check files of mime-type text/* for svn:eol-style property.
+my $check_text_eol = 1;
+
+# Toggle: Check property-modified files too.
+my $check_prop_modified_files = 0;
+
# Svnlook path.
my $svnlook = "/usr/bin/svnlook";
@@ -100,19 +114,28 @@ my $tmp_dir = '/tmp';
chdir($tmp_dir)
or die "$0: cannot chdir `$tmp_dir': $!\n";
-# Figure out what files have added using svnlook.
-my @files_added;
+# Figure out what files have been added/property-modified using svnlook.
+my $regex_files_to_check;
+if ($check_prop_modified_files)
+ {
+ $regex_files_to_check = qr/^(?:A.|.U) (.*[^\/])$/;
+ }
+else
+ {
+ $regex_files_to_check = qr/^A. (.*[^\/])$/;
+ }
+my @files_to_check;
foreach my $line (&read_from_process($svnlook, 'changed', $repos, '-t', $txn))
{
- # Add only files that were added to @files_added
- if ($line =~ /^A. (.*[^\/])$/)
+ # Add only files that were added/property-modified to @files_to_check
+ if ($line =~ /$regex_files_to_check/)
{
- push(@files_added, $1);
+ push(@files_to_check, $1);
}
}
my @errors;
-foreach my $path ( @files_added )
+foreach my $path ( @files_to_check )
{
my $mime_type;
my $eol_style;
@@ -168,7 +191,7 @@ foreach my $path ( @files_added )
{
push @errors, "$path : svn:mime-type is not set";
}
- elsif ($mime_type =~ /^text\// and not $eol_style)
+ elsif ($check_text_eol and $mime_type =~ /^text\// and not $eol_style)
{
push @errors, "$path : svn:mime-type=$mime_type but svn:eol-style is not set";
}
@@ -179,20 +202,30 @@ foreach my $path ( @files_added )
# and will not see this verbose message more than once.
if (@errors)
{
+ my $addition1 = '';
+ my $addition2 = '';
+ my $addition3 = '';
+ if ($check_prop_modified_files)
+ {
+ $addition1 = '/property-modified';
+ }
+ if ($check_text_eol)
+ {
+ $addition2 = " In addition text files must have the svn:eol-style property set.\n";
+ $addition3 = " svn propset svn:eol-style native path/of/file\n";
+ }
warn "$0:\n\n",
join("\n", @errors), "\n\n",
- <<EOS;
-
- Every added file must have the svn:mime-type property set. In
- addition text files must have the svn:eol-style property set.
+ <<"EOS";
+ Every added$addition1 file must have the svn:mime-type property set.
+$addition2
For binary files try running
svn propset svn:mime-type application/octet-stream path/of/file
For text files try
svn propset svn:mime-type text/plain path/of/file
- svn propset svn:eol-style native path/of/file
-
+$addition3
You may want to consider uncommenting the auto-props section
in your ~/.subversion/config file. Read the Subversion book
(http://svnbook.red-bean.com/), Chapter 7, Properties section,
#!/usr/bin/env perl
# ====================================================================
# check-mime-type.pl: check that every added or property-modified file
# has the svn:mime-type property set and every added or property-modified
# file with a mime-type matching text/* also has svn:eol-style set.
# If any file fails this test the user is sent a verbose error message
# suggesting solutions and the commit is aborted.
#
# Usage: check-mime-type.pl REPOS TXN-NAME
# ====================================================================
# Most of check-mime-type.pl was taken from
# commit-access-control.pl, Revision 9986, 2004-06-14 16:29:22 -0400.
# ====================================================================
# Copyright (c) 2000-2009 CollabNet. All rights reserved.
# Copyright (c) 2010-2020 Apache Software Foundation (ASF).
# ====================================================================
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
# ====================================================================
# Turn on warnings the best way depending on the Perl version.
BEGIN {
if ( $] >= 5.006_000)
{ require warnings; import warnings; }
else
{ $^W = 1; }
}
use strict;
use Carp;
######################################################################
# Configuration section.
# Toggle: Check files of mime-type text/* for svn:eol-style property.
my $check_text_eol = 1;
# Toggle: Check property-modified files too.
my $check_prop_modified_files = 0;
# Svnlook path.
my $svnlook = "/usr/bin/svnlook";
# Since the path to svnlook depends upon the local installation
# preferences, check that the required program exists to insure that
# the administrator has set up the script properly.
{
my $ok = 1;
foreach my $program ($svnlook)
{
if (-e $program)
{
unless (-x $program)
{
warn "$0: required program `$program' is not executable, ",
"edit $0.\n";
$ok = 0;
}
}
else
{
warn "$0: required program `$program' does not exist, edit $0.\n";
$ok = 0;
}
}
exit 1 unless $ok;
}
######################################################################
# Initial setup/command-line handling.
&usage unless @ARGV == 2;
my $repos = shift;
my $txn = shift;
unless (-e $repos)
{
&usage("$0: repository directory `$repos' does not exist.");
}
unless (-d $repos)
{
&usage("$0: repository directory `$repos' is not a directory.");
}
# Define two constant subroutines to stand for read-only or read-write
# access to the repository.
sub ACCESS_READ_ONLY () { 'read-only' }
sub ACCESS_READ_WRITE () { 'read-write' }
######################################################################
# Harvest data using svnlook.
# Change into /tmp so that svnlook diff can create its .svnlook
# directory.
my $tmp_dir = '/tmp';
chdir($tmp_dir)
or die "$0: cannot chdir `$tmp_dir': $!\n";
# Figure out what files have been added/property-modified using svnlook.
my $regex_files_to_check;
if ($check_prop_modified_files)
{
$regex_files_to_check = qr/^(?:A.|.U) (.*[^\/])$/;
}
else
{
$regex_files_to_check = qr/^A. (.*[^\/])$/;
}
my @files_to_check;
foreach my $line (&read_from_process($svnlook, 'changed', $repos, '-t', $txn))
{
# Add only files that were added/property-modified to @files_to_check
if ($line =~ /$regex_files_to_check/)
{
push(@files_to_check, $1);
}
}
my @errors;
foreach my $path ( @files_to_check )
{
my $mime_type;
my $eol_style;
# Parse the complete list of property values of the file $path
to extract
# the mime-type and eol-style
my @output = &read_from_process($svnlook, 'proplist', $repos,
'-t',
$txn, '--verbose', '--', $path);
my $output_line = 0;
foreach my $prop (@output)
{
if ($prop =~ /^\s*svn:mime-type( : (\S+))?/)
{
$mime_type = $2;
# 1.7.8 (r1416637) onwards
changed the format of svnloop proplist --verbose
# from propname : propvalue
format, to values in an indent list on following lines
if (not $mime_type)
{
if
($output_line + 1 >= scalar(@output))
{
die "$0: Unexpected EOF reading proplist.\n";
}
my
$next_line_pval_indented = $output[$output_line + 1];
if
($next_line_pval_indented =~ /^\s{4}(.*)/)
{
$mime_type = $1;
}
}
}
elsif ($prop =~ /^\s*svn:eol-style( : (\S+))?/)
{
$eol_style = $2;
if (not $eol_style)
{
if
($output_line + 1 >= scalar(@output))
{
die "$0: Unexpected EOF reading proplist.\n";
}
my
$next_line_pval_indented = $output[$output_line + 1];
if
($next_line_pval_indented =~ /^\s{4}(.*)/)
{
$eol_style = $1;
}
}
}
$output_line++;
}
# Detect error conditions and add them to @errors
if (not $mime_type)
{
push @errors, "$path : svn:mime-type is not
set";
}
elsif ($check_text_eol and $mime_type =~ /^text\// and not
$eol_style)
{
push @errors, "$path : svn:mime-type=$mime_type
but svn:eol-style is not set";
}
}
# If there are any errors list the problem files and give information
# on how to avoid the problem. Hopefully people will set up auto-props
# and will not see this verbose message more than once.
if (@errors)
{
my $addition1 = '';
my $addition2 = '';
my $addition3 = '';
if ($check_prop_modified_files)
{
$addition1 = '/property-modified';
}
if ($check_text_eol)
{
$addition2 = " In addition text files must have the svn:eol-style
property set.\n";
$addition3 = " svn propset svn:eol-style native path/of/file\n";
}
warn "$0:\n\n",
join("\n", @errors), "\n\n",
<<"EOS";
Every added$addition1 file must have the svn:mime-type property set.
$addition2
For binary files try running
svn propset svn:mime-type application/octet-stream path/of/file
For text files try
svn propset svn:mime-type text/plain path/of/file
$addition3
You may want to consider uncommenting the auto-props section
in your ~/.subversion/config file. Read the Subversion book
(http://svnbook.red-bean.com/), Chapter 7, Properties section,
Automatic Property Setting subsection for more help.
EOS
exit 1;
}
else
{
exit 0;
}
sub usage
{
warn "@_\n" if @_;
die "usage: $0 REPOS TXN-NAME\n";
}
sub safe_read_from_pipe
{
unless (@_)
{
croak "$0: safe_read_from_pipe passed no arguments.\n";
}
print "Running @_\n";
my $pid = open(SAFE_READ, '-|', @_);
unless (defined $pid)
{
die "$0: cannot fork: $!\n";
}
unless ($pid)
{
open(STDERR, ">&STDOUT")
or die "$0: cannot dup STDOUT: $!\n";
exec(@_)
or die "$0: cannot exec `@_': $!\n";
}
my @output;
while (<SAFE_READ>)
{
chomp;
push(@output, $_);
}
close(SAFE_READ);
my $result = $?;
my $exit = $result >> 8;
my $signal = $result & 127;
my $cd = $result & 128 ? "with core dump" : "";
if ($signal or $cd)
{
warn "$0: pipe from `@_' failed $cd: exit=$exit signal=$signal\n";
}
if (wantarray)
{
return ($result, @output);
}
else
{
return $result;
}
}
sub read_from_process
{
unless (@_)
{
croak "$0: read_from_process passed no arguments.\n";
}
my ($status, @output) = &safe_read_from_pipe(@_);
if ($status)
{
if (@output)
{
die "$0: `@_' failed with this output:\n", join("\n", @output), "\n";
}
else
{
die "$0: `@_' failed with no output.\n";
}
}
else
{
return @output;
}
}
