Merge pull request #893 from grahamc/fake-channel-nested

Test the fake derivations channel, asserting nested packages are properly represented.

src/lib/Hydra/Helper/AttributeSet.pm

@@ -0,0 +1,56 @@
+package Hydra::Helper::AttributeSet;
+
+use strict;
+use warnings;
+
+sub new {
+    my ($self) = @_;
+    return bless { "paths" => [] }, $self;
+}
+
+sub registerValue {
+    my ($self, $attributePath) = @_;
+
+    my @pathParts = splitPath($attributePath);
+
+    pop(@pathParts);
+    if (scalar(@pathParts) == 0) {
+        return;
+    }
+
+    my $lineage = "";
+    for my $pathPart (@pathParts) {
+        $lineage = $self->registerChild($lineage, $pathPart);
+    }
+}
+
+sub registerChild {
+    my ($self, $parent, $attributePath) = @_;
+    if ($parent ne "") {
+        $parent .= "."
+    }
+
+    my $name = $parent . $attributePath;
+    if (!grep { $_ eq $name} @{$self->{"paths"}}) {
+        push(@{$self->{"paths"}}, $name);
+    }
+    return $name;
+}
+
+sub splitPath {
+    my ($s) = @_;
+
+    if ($s eq "") {
+        return ('')
+    }
+
+    return split(/\./, $s, -1);
+}
+
+sub enumerate {
+    my ($self) = @_;
+    my @paths = sort { length($a) <=> length($b) } @{$self->{"paths"}};
+    return wantarray ? @paths : \@paths;
+}
+
+1;

src/lib/Hydra/Helper/Escape.pm

@@ -0,0 +1,21 @@
+package Hydra::Helper::Escape;
+
+use strict;
+use base qw(Exporter);
+use Hydra::Helper::AttributeSet;
+
+our @EXPORT = qw(escapeString escapeAttributePath);
+
+sub escapeString {
+    my ($s) = @_;
+    $s =~ s|\\|\\\\|g;
+    $s =~ s|\"|\\\"|g;
+    $s =~ s|\$|\\\$|g;
+    return "\"" . $s . "\"";
+}
+
+sub escapeAttributePath {
+    my ($s) = @_;
+
+    return join ".", map { escapeString($_) } Hydra::Helper::AttributeSet::splitPath($s);
+}

src/lib/Hydra/View/NixExprs.pm

@@ -3,18 +3,12 @@ package Hydra::View::NixExprs;
 use strict;
 use base qw/Catalyst::View/;
 use Hydra::Helper::Nix;
+use Hydra::Helper::Escape;
+use Hydra::Helper::AttributeSet;
 use Archive::Tar;
 use IO::Compress::Bzip2 qw(bzip2);
 use Encode;
-
-
-sub escape {
-    my ($s) = @_;
-    $s =~ s|\\|\\\\|g;
-    $s =~ s|\"|\\\"|g;
-    $s =~ s|\$|\\\$|g;
-    return "\"" . $s . "\"";
-}
+use Data::Dumper;
 
 
 sub process {
@@ -62,33 +56,38 @@ EOF
     my $first = 1;
     foreach my $system (keys %perSystem) {
         $res .= "else " if !$first;
-        $res .= "if system == ${\escape $system} then {\n\n";
-
+        $res .= "if system == ${\escapeString $system} then {\n\n";
+        my $attrsets = Hydra::Helper::AttributeSet->new();
         foreach my $job (keys %{$perSystem{$system}}) {
             my $pkg = $perSystem{$system}->{$job};
             my $build = $pkg->{build};
-            $res .= "  # Hydra build ${\$build->id}\n";
             my $attr = $build->get_column('job');
-            $attr =~ s/\./-/g;
-            $res .= "  ${\escape $attr} = (mkFakeDerivation {\n";
+            $attrsets->registerValue($attr);
+
+            $res .= "  # Hydra build ${\$build->id}\n";
+            $res .= "  ${\escapeAttributePath $attr} = (mkFakeDerivation {\n";
             $res .= "    type = \"derivation\";\n";
-            $res .= "    name = ${\escape ($build->get_column('releasename') or $build->nixname)};\n";
-            $res .= "    system = ${\escape $build->system};\n";
+            $res .= "    name = ${\escapeString ($build->get_column('releasename') or $build->nixname)};\n";
+            $res .= "    system = ${\escapeString $build->system};\n";
             $res .= "    meta = {\n";
-            $res .= "      description = ${\escape $build->description};\n"
+            $res .= "      description = ${\escapeString $build->description};\n"
                 if $build->description;
-            $res .= "      license = ${\escape $build->license};\n"
+            $res .= "      license = ${\escapeString $build->license};\n"
                 if $build->license;
-            $res .= "      maintainers = ${\escape $build->maintainers};\n"
+            $res .= "      maintainers = ${\escapeString $build->maintainers};\n"
                 if $build->maintainers;
             $res .= "    };\n";
             $res .= "  } {\n";
             my @outputNames = sort (keys %{$pkg->{outputs}});
-            $res .= "    ${\escape $_} = ${\escape $pkg->{outputs}->{$_}};\n" foreach @outputNames;
+            $res .= "    ${\escapeString $_} = ${\escapeString $pkg->{outputs}->{$_}};\n" foreach @outputNames;
             my $out = defined $pkg->{outputs}->{"out"} ? "out" : $outputNames[0];
             $res .= "  }).$out;\n\n";
         }
 
+        for my $attrset ($attrsets->enumerate()) {
+            $res .= "  ${\escapeAttributePath $attrset}.recurseForDerivations = true;\n\n";
+        }
+
         $res .= "}\n\n";
         $first = 0;
     }

t/Controller/Jobset/channel.t

@@ -0,0 +1,62 @@
+use feature 'unicode_strings';
+use strict;
+use Setup;
+use IO::Uncompress::Bunzip2 qw(bunzip2);
+use Archive::Tar;
+use JSON qw(decode_json);
+use Data::Dumper;
+my %ctx = test_init();
+
+require Hydra::Schema;
+require Hydra::Model::DB;
+require Hydra::Helper::Nix;
+
+use Test2::V0;
+require Catalyst::Test;
+Catalyst::Test->import('Hydra');
+
+my $db = Hydra::Model::DB->new;
+hydra_setup($db);
+
+my $project = $db->resultset('Projects')->create({name => "tests", displayname => "", owner => "root"});
+
+# Most basic test case, no parameters
+my $jobset = createBaseJobset("nested-attributes", "nested-attributes.nix", $ctx{jobsdir});
+
+ok(evalSucceeds($jobset));
+is(nrQueuedBuildsForJobset($jobset), 4);
+
+for my $build (queuedBuildsForJobset($jobset)) {
+    ok(runBuild($build), "Build '".$build->job."' should exit with code 0");
+    my $newbuild = $db->resultset('Builds')->find($build->id);
+    is($newbuild->finished, 1, "Build '".$build->job."' should be finished.");
+    is($newbuild->buildstatus, 0, "Build '".$build->job."' should have buildstatus 0.");
+}
+
+my $compressed = get('/jobset/tests/nested-attributes/channel/latest/nixexprs.tar.bz2');
+my $tarcontent;
+bunzip2(\$compressed => \$tarcontent);
+open(my $tarfh, "<", \$tarcontent);
+my $tar = Archive::Tar->new($tarfh);
+
+my $defaultnix = $ctx{"tmpdir"} . "/channel-default.nix";
+$tar->extract_file("channel/default.nix", $defaultnix);
+
+print STDERR $tar->get_content("channel/default.nix");
+
+(my $status, my $stdout, my $stderr) = Hydra::Helper::Nix::captureStdoutStderr(5, "nix-env", "--json", "--query", "--available", "--attr-path", "--file", $defaultnix);
+is($stderr, "", "Stderr should be empty");
+is($status, 0, "Querying the packages should succeed");
+
+my $packages = decode_json($stdout);
+my $keys = [sort keys %$packages];
+is($keys, [
+    "packageset-nested",
+    "packageset.deeper.deeper.nested",
+    "packageset.nested",
+    "packageset.nested2",
+]);
+is($packages->{"packageset-nested"}->{"name"}, "actually-top-level");
+is($packages->{"packageset.nested"}->{"name"}, "actually-nested");
+
+done_testing;

t/Helper/attributeset.t

@@ -0,0 +1,53 @@
+use strict;
+use warnings;
+use Setup;
+use Data::Dumper;
+use Test2::V0;
+use Hydra::Helper::AttributeSet;
+
+
+subtest "splitting an attribute path in to its component parts" => sub {
+    my %values = (
+        "" => [''],
+        "." => ['', ''],
+        "...." => ['', '', '', '', ''],
+        "foobar" => ['foobar'],
+        "foo.bar" => ['foo', 'bar'],
+        "🌮" => ['🌮'],
+
+        # not supported: 'foo."bar.baz".tux' => [ 'foo', 'bar.baz', 'tux' ]
+        # the edge cases are fairly significant around escaping and unescaping.
+    );
+
+    for my $input (keys %values) {
+        my @value = @{$values{$input}};
+        my @components = Hydra::Helper::AttributeSet::splitPath($input);
+        is(\@components, \@value, "Splitting the attribute path: " . $input);
+    }
+};
+
+my $attrs = Hydra::Helper::AttributeSet->new();
+$attrs->registerValue("foo");
+$attrs->registerValue("bar.baz.tux");
+$attrs->registerValue("bar.baz.bux.foo.bar.baz");
+
+is(
+    $attrs->enumerate(),
+    [
+        # "foo": skipped since we're registering values, and we
+        # only want to track nested attribute sets.
+
+        # "bar.baz.tux": expand the path
+        "bar",
+        "bar.baz",
+
+        #"bar.baz.bux.foo.bar.baz": expand the path, but only register new
+        # attribute set names.
+        "bar.baz.bux",
+        "bar.baz.bux.foo",
+        "bar.baz.bux.foo.bar",
+    ],
+    "Attribute set paths are registered."
+);
+
+done_testing;

t/Helper/escape.t

@@ -0,0 +1,44 @@
+use strict;
+use Setup;
+use Data::Dumper;
+use Test2::V0;
+use Hydra::Helper::Escape;
+
+subtest "checking individual attribute set elements" => sub {
+    my %values = (
+        "" => '""',
+        "." => '"."',
+        "foobar" => '"foobar"',
+        "foo.bar" => '"foo.bar"',
+        "🌮" => '"🌮"',
+        'foo"bar' => '"foo\"bar"',
+        'foo\\bar' => '"foo\\\\bar"',
+        '$bar' => '"\\$bar"',
+    );
+
+    for my $input (keys %values) {
+        my $value = $values{$input};
+        is(escapeString($input), $value, "Escaping the value: " . $input);
+    }
+};
+
+subtest "escaping path components of a nested attribute" => sub {
+    my %values = (
+        "" => '""',
+        "." => '"".""',
+        "...." => '""."".""."".""',
+        "foobar" => '"foobar"',
+        "foo.bar" => '"foo"."bar"',
+        "🌮" => '"🌮"',
+        'foo"bar' => '"foo\"bar"',
+        'foo\\bar' => '"foo\\\\bar"',
+        '$bar' => '"\\$bar"',
+    );
+
+    for my $input (keys %values) {
+        my $value = $values{$input};
+        is(escapeAttributePath($input), $value, "Escaping the attribute path: " . $input);
+    }
+};
+
+done_testing;

t/jobs/nested-attributes.nix

@@ -0,0 +1,36 @@
+with import ./config.nix;
+rec {
+  # Given a jobset containing a package set named X with an interior member Y,
+  # expose the interior member Y with the name X-Y. This is to exercise a bug
+  # in the NixExprs view's generated Nix expression which flattens the
+  # package set namespace from `X.Y` to `X-Y`. If the bug is present, the
+  # resulting expression incorrectly renders two `X-Y` packages.
+  packageset = {
+    recurseForDerivations = true;
+    deeper = {
+      recurseForDerivations = true;
+      deeper = {
+        recurseForDerivations = true;
+
+        nested = mkDerivation {
+          name = "much-too-deep";
+          builder = ./empty-dir-builder.sh;
+        };
+      };
+    };
+
+    nested = mkDerivation {
+      name = "actually-nested";
+      builder = ./empty-dir-builder.sh;
+    };
+
+    nested2 = mkDerivation {
+      name = "actually-nested2";
+      builder = ./empty-dir-builder.sh;
+    };
+  };
+  packageset-nested = mkDerivation {
+    name = "actually-top-level";
+    builder = ./empty-dir-builder.sh;
+  };
+}