Skip to content

Commit

Permalink
[io grant] Improve &chdir, &indir, and IO::Path.chdir
Browse files Browse the repository at this point in the history
- Deprecate `:$test` param
- Use :r, :w, :d, :x named args for tests
- Use :d test as default instead of :d + :r
- Make &chdir and &indir propagate Failure, instead of throwing
- Fix incorrect dir used in &indir if a relative IO::Path is given
    after a change to $*CWD, since IO::Path.Str does not use its $!CWD
- Fix (or reduce?) race condition in `indir` for relative paths:
    - old version took Str() $path and used $*CWD in its body to
        chdir to that $path, creating a race condition where the
        changed $*CWD made $path be relative to the wrong thing
    - new version takes IO(), so no race if an IO::Path is given
        and a bit smaller race if a Str is given, as the $*CWD is used
        during coersion and not in body
  • Loading branch information
zoffixznet committed Apr 2, 2017
1 parent 3236823 commit a0ef2ed
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 40 deletions.
59 changes: 33 additions & 26 deletions src/core/IO/Path.pm
Original file line number Diff line number Diff line change
Expand Up @@ -239,8 +239,18 @@ my class IO::Path is Cool {
}

proto method chdir(|) { * }
multi method chdir(IO::Path:D: Str() $path is copy, :$test = 'r') {
if !$!SPEC.is-absolute($path) {
multi method chdir(IO::Path:D: Str() $path, :$test!) {
DEPRECATED(
:what<:$test argument>,
'individual named parameters (e.g. :r, :w, :x)',
"v2017.03.101.ga.5800.a.1", "v6.d", :up(*),
);
self.chdir: $path, |$test.words.map(* => True).Hash;
}
multi method chdir(
IO::Path:D: Str() $path is copy, :$d = True, :$r, :$w, :$x,
) {
unless $!SPEC.is-absolute($path) {
my ($volume,$dirs) = $!SPEC.splitpath(self.abspath, :nofile);
my @dirs = $!SPEC.splitdir($dirs);
@dirs.shift; # the first is always empty for absolute dirs
Expand All @@ -257,30 +267,27 @@ my class IO::Path is Cool {
}
my $dir = IO::Path.new-from-absolute-path($path,:$!SPEC,:CWD(self));

# basic sanity
unless $dir.d {
fail X::IO::Chdir.new(
:$path,
:os-error( $dir.e
?? "is not a directory"
!! "does not exist"),
);
}

if $test eq 'r' {
return $dir if $dir.r;
}
elsif $test eq 'r w' {
return $dir if $dir.r and $dir.w;
}
elsif $test eq 'r w x' {
return $dir if $dir.r and $dir.w and $dir.x;
}

fail X::IO::Chdir.new(
:$path,
:os-error("did not pass 'd $test' test"),
);
nqp::stmts(
nqp::unless(
nqp::unless(nqp::isfalse($d), $dir.d),
fail X::IO::Chdir.new: :$path, :os-error(
nqp::if($dir.e, 'is not a directory', 'does not exist')
)
),
nqp::unless(
nqp::unless(nqp::isfalse($r), $dir.r),
fail X::IO::Chdir.new: :$path, :os-error("did not pass :r test")
),
nqp::unless(
nqp::unless(nqp::isfalse($w), $dir.w),
fail X::IO::Chdir.new: :$path, :os-error("did not pass :w test")
),
nqp::unless(
nqp::unless(nqp::isfalse($x), $dir.x),
fail X::IO::Chdir.new: :$path, :os-error("did not pass :x test")
),
$dir
)
}

proto method rename(|) { * }
Expand Down
24 changes: 10 additions & 14 deletions src/core/io_operators.pm
Original file line number Diff line number Diff line change
Expand Up @@ -161,20 +161,16 @@ multi sub spurt(Cool $path, $contents, |c) {
PROCESS::<&chdir> := &chdir;
}

sub chdir(Str() $path, :$test = 'r') {
my $newCWD := $*CWD.chdir($path,:$test);
$newCWD // $newCWD.throw;

$*CWD = $newCWD;
}

sub indir(Str() $path, $what, :$test = <r w>) {
my $newCWD := $*CWD.chdir($path,:$test);
$newCWD // $newCWD.throw;

{
my $*CWD = $newCWD; # temp doesn't work in core settings :-(
$what();
sub chdir(|c) { $*CWD .= chdir(|c) }
sub indir(IO() $path, &what, |c) {
{ # NOTE: we need this extra block so that the IO() coercer doesn't
# use our (empty at the time) $*CWD when making the IO::Path object

# We call .chdir for the sake of its doing the :d:r:w:x tests
nqp::if(
nqp::istype((my $*CWD = $path.chdir($path,|c)), Failure),
$*CWD, what,
)
}
}

Expand Down

0 comments on commit a0ef2ed

Please sign in to comment.