Skip to content

Commit

Permalink
πŸ’₯🀯 Great refactoring, many tests, IO task rework, etc.
Browse files Browse the repository at this point in the history
Affected tickets:
* Resolves #7: current task is now passed into the block (for both
  normal and IO tasks), with tests.
* Resolves #12: using `run` in sink context now explodes properly
  (tested for both normal and IO tasks).
* Resolves #13: no more shell injection through filenames, tested with
  extra tests for filenames starting with `--`.
* Resolves #16: there are now helper functions to test sakefiles
  without the need to `.execute` tasks from within the test file.  It
  works by creating a temp directory with a given Sakefile and running
  `sake` as external command (stdout, stderr, exit code and signals
  are checked).
* Resolves #18: you can now pass an IO object instead of a Str and it
  will automatically dispatch to the right sub.
* There is some groundwork for #17, and maybe it already works. I
  can't tell if it does because there are no tests for it yet.
* Also some minimal groundwork for #9 (parallel execution).
* Additionally, issue #14 should be more approachable now (because
  deps are now resolved).
* Issue #15 (β€˜default’ task) was reworked a bit (with no notable
  functional changes).

This commit splits Sake.pm6 into separate files for convenience. The
hierarchy is perhaps not entirely right, but now it is much easier to
refactor it further with all the tests.

Because almost all lines were touched, this commit introduces some
major code style changes (to my personal preference, e.g. unicode
quotes). I'm not insisting on that style, so if anybody cares enough
it should be possible to submit a PR with unicode stuff autoreplaced.

I wish this commit was split into multiple digestable commits, but it
was a single refactoring effort that I ended up shelving for a few
months anyway, so I'm happy that it goes in at all.
  • Loading branch information
AlexDaniel committed Oct 6, 2018
1 parent 4972a25 commit 8d54732
Show file tree
Hide file tree
Showing 22 changed files with 711 additions and 96 deletions.
19 changes: 11 additions & 8 deletions bin/sake
Expand Up @@ -3,13 +3,16 @@
use v6;
use Sake;

sub MAIN (*@tasks, :$file = 'Sakefile') {
die "Could not find file $file!" unless $file.IO ~~ :e;
EVALFILE $file;
if @tasks < 1 {
note β€˜No tasks given! Did you mean one of these?’;
note .indent: 4 for %Sake::TASKS.keys.sort;
exit 1
sub MAIN(*@tasks,
:$file = β€˜Sakefile’,
:$force = False,
) {

if !$file.IO.e {
note β€œCould not find file $file!”;
exit 2
}
for @tasks -> $t { execute($t); }
EVALFILE $file;
sake-precheck :$force;
execute @tasks || β€˜default’
}
104 changes: 42 additions & 62 deletions lib/Sake.pm6
@@ -1,73 +1,53 @@
unit module Sake;

our %TASKS;

class Sake-Task {
has $.name = !!! 'name required';
has @.deps; # dependencies
has &.body = !!! 'body required'; # code to execute for this task
has &.cond = { True }; # only execute when True

method execute {
return unless self.cond.();
for self.deps -> $d { execute($d); }
.(self) with self.body;
}

}

sub execute($task) is export {
if %TASKS{$task}:exists {
sink %TASKS{$task}.execute;
} else {
# TODO something more awesome here
$*ERR.say("No task named $task...skipping");
}
use Sake::Task;
use Sake::TaskStore;
use Sake::Task::IO;

sub EXPORT {
%(
Sake::Task::EXPORT::DEFAULT::,
Sake::Task::IO::EXPORT::DEFAULT::,
)
}

proto sub task(|) is export { * }

my sub make-task($name, &body, :@deps=[], :&cond={True}) {
die "Duplicate task $name!" if %TASKS{~$name};
%TASKS{~$name} = Sake-Task.new(:$name, :&body, :@deps, :&cond);
}
unit module Sake;

multi sub task(Str $name, &body) {
make-task($name, &body);
multi execute(Str $task) {
if %TASKS{$task}:!exists {
note β€œTask β€œ$task” does not exist”;
note did-you-mean;
exit 2
}
execute %TASKS{$task}
}

multi sub task(Pair $name-deps, &body?) {
my ($name,$deps) := $name-deps.kv; # unpack name and dependencies
my @deps = $deps.list; # so that A => B and A => <B C D> both work
return make-task($name, &body, :@deps);
multi execute(Sake::Task $task) {
my $result = $task.execute;
$result ~~ Promise
?? await $result
!! $result
}


proto sub file(|) is export { * }

my sub touch (Str $filename) {
run β€˜touch’, β€˜--’, $filename;
multi execute(*@tasks) is export {
my @non-existent = @tasks.grep: { %TASKS{$_}:!exists };
if @non-existent {
note β€œTask β€œ$_” does not exist” for @non-existent;
note did-you-mean;
exit 2
}
(execute $_ for @tasks)
}

multi sub file(Str $name, &body) {
return make-task(
$name,
{ &body($_); touch $name },
:cond(sub { $name.path !~~ :e; })
)
}
sub sake-precheck(:$force = False) is export {
my @errors = gather resolve-deps;
if @errors {
.note for @errors;

multi sub file(Pair $name-deps, &body) {
my ($name,$deps) := $name-deps.kv; # unpack name and dependencies
my @deps = $deps.list; # so that A => B and A => <B C D> both work
my $cond = {
my $f = $name.path;
!($f ~~ :e) || $f.modified < all(map { $_.modified }, grep { $_ ~~ IO::Path }, @deps);
};
return make-task(
$name,
{ &body($_); touch $name },
:@deps,
:cond($cond)
)
if $force {
note β€˜β€™;
note β€˜Continuing with errors because of the --force flag’;
} else {
note β€˜Exiting. Use --force flag if you want to ignore these errors’;
exit 1
}
}
}
72 changes: 72 additions & 0 deletions lib/Sake/Task.pm6
@@ -0,0 +1,72 @@
use Sake::TaskStore;

unit class Sake::Task;

has $.name = !!! β€˜name required’;
has @.deps; #= Task dependencies
has &.body = !!! β€˜body required’; #= Code to execute for this task
has &.cond = { True }; #= Condition for task execution
has $.modification-time = -∞;
has $.ready = Promise.new;
has $.callframe;

#| Executes the task, even if it was already executed.
method execute {
if $.ready.status !~~ Planned {
note β€œWarning: re-executing task β€œ$.name” per your request”
}
resolve-deps self, :live; # in case the task was added later
await @.depsΒ».readify;

if $.cond() {
with self.body {
sink .signature.ACCEPTS(\(self))
?? .(self)
!! .();
}
$!modification-time = now;
}

$.ready.keep: $.modification-time unless $.ready.status ~~ Kept;
}

method readify {
# TODO race conditions
self.execute unless $.ready.status ~~ Kept;
$.ready
}


multi resolve-deps($task, :$live = False) is export {
$task.deps .= map: {
do if $_ ~~ Sake::Task {
$_ # already resolved
} elsif %TASKS{$_}:exists {
%TASKS{$_}
} else {
my $msg = β€œTask $task.name() depends on $_ but no such task was found”;
$live ?? note $msg !! take $msg;
Empty
}
}
}

multi resolve-deps is export {
my @errors = gather { resolve-deps $_ for %TASKS.values }
if @errors > 0 { # TODO what if it's another error
take $_ for @errors;
take did-you-mean
}
}



proto sub task(|) is export {*}

multi sub task(Str $name, &body?) {
make-task $name, &body, type => Sake::Task
}

multi sub task(Pair (Str :key($name), :value($deps)), &body?) {
make-task $name, &body, type => Sake::Task, deps => $deps.list
}
48 changes: 48 additions & 0 deletions lib/Sake/Task/IO.pm6
@@ -0,0 +1,48 @@
use Sake::Task;
use Sake::TaskStore;

unit class Sake::Task::IO is Sake::Task;

method modification-time {
$.name.IO.e
?? $.name.IO.modified
!! -∞
}

method execute {
resolve-deps self, :live; # in case the task was added later
await @.depsΒ».readify;

if $.cond() {
with self.body {
my $last-dep = @.depsΒ».modification-time.max;
$last-dep = now if $last-dep == -∞;
sink .(self) if $.modification-time < $last-dep;
}

my &touch = -> $filename { run <touch -->, $filename };
touch $.name.IO;
}
$.ready.keep: $.modification-time unless $.ready;
}



multi sub task(IO $path, &body?) is export {
make-task $path, &body, type => Sake::Task::IO
}

multi sub task(Pair (IO :key($path), :value($deps)), &body?) is export {
make-task $path, &body, deps => $deps.list, type => Sake::Task::IO
}


proto sub file(|) is export {*}

multi sub file(IO() $path, &body?) {
task $path, &body
}

multi sub file(Pair (IO() :key($path), :value($deps)), &body?) {
task $path => $deps, &body
}
19 changes: 19 additions & 0 deletions lib/Sake/TaskStore.pm6
@@ -0,0 +1,19 @@
unit module Sake::TaskStore;

our %TASKS is export;

sub make-task($name, &body,
:@deps=[], :&cond={True}, :$type) is export {
if %TASKS{$name}:exists {
note β€œDuplicate task $name”;
exit 2
}
%TASKS{~$name} = $type.new: :$name, :&body, :@deps,
:&cond, callframe => callframe
}

sub did-you-mean is export {
return β€œ\nNo tasks were defined” if %TASKS == 0;
β€œ\nDid you mean one of these?\n”
~ %TASKS.keys.sortΒ».indent(4).join: β€œ\n”
}
26 changes: 0 additions & 26 deletions t/00-basic.t

This file was deleted.

26 changes: 26 additions & 0 deletions t/00-original-task.t
@@ -0,0 +1,26 @@
use v6;
use lib <lib/ t/lib>;
use Sake;
use Test;

plan 5;

my $x = β€˜β€™;

my $t = task β€˜fred’, { $x = β€˜meth’ }
$t.execute;
is $x, β€˜meth’, β€˜can execute task via method’;

$x = β€˜β€™;
execute β€˜fred’;
is $x, β€˜meth’, β€˜can execute task by name’;

(task β€˜dino’ => β€˜fred’, { $x = β€˜sd’ }).execute;
is $x, β€˜sd’, β€˜single dependency works fine’;

(task β€˜bedrock’ => <fred dino>, { $x = β€˜md’ }).execute;
is $x, β€˜md’, β€˜multiple dependencies work fine’;

task β€˜clear-headed-fred’, { $x = β€˜again’ }
(task β€˜wilma’ => <clear-headed-fred>).execute;
is $x, β€˜again’, β€˜body is optional’;
34 changes: 34 additions & 0 deletions t/01-original-file.t
@@ -0,0 +1,34 @@
use v6;
use lib <lib/ t/lib>;
use Sake;
use Test;

plan 11;

my $x = β€˜β€™;

my $t = file β€˜fred’, { $x = β€˜meth’ }
$t.execute;
is $x, β€˜meth’, β€˜can execute file task via method’;
ok β€˜fred’.IO.e, β€˜file exists’;

$x = β€˜β€™;
β€˜fred’.IO.unlink;

execute β€˜fred’;
is $x, β€˜meth’, β€˜can execute file task by name’;
ok β€˜fred’.IO.e, β€˜file exists’;

(file β€˜dino’ => β€˜fred’, { $x = β€˜sd’ }).execute;
is $x, β€˜sd’, β€˜single file dependency works fine’;
ok β€˜dino’.IO.e, β€˜file exists’;

(file β€˜bedrock’ => <fred dino>, { $x = β€˜md’ }).execute;
is $x, β€˜md’, β€˜multiple file dependencies work fine’;
ok β€˜bedrock’.IO.e, β€˜file exists’;

file β€˜clear-headed-fred’, { $x = β€˜again’ }
(file β€˜wilma’ => <clear-headed-fred>).execute;
is $x, β€˜again’, β€˜body is optional’;
ok β€˜clear-headed-fred’.IO.e, β€˜file exists’;
ok β€˜wilma’.IO.e, β€˜file exists’;

0 comments on commit 8d54732

Please sign in to comment.