Merge pull request #106 from NixOS/can-eval

Don't try and build stuff that cannot be built

ofborg/Cargo.lock

@@ -379,7 +379,7 @@ dependencies = [
 
 [[package]]
 name = "ofborg"
-version = "0.1.1"
+version = "0.1.2"
 dependencies = [
  "amqp 0.1.0 (git+https://github.com/grahamc/rust-amqp.git)",
  "either 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)",

ofborg/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "ofborg"
-version = "0.1.1"
+version = "0.1.2"
 authors = ["Graham Christensen <graham@grahamc.com>"]
 include = ["Cargo.toml", "Cargo.lock", "src", "test-srcs", "build.rs"]
 build = "build.rs"

ofborg/src/bin/log-message-generator.rs

@@ -50,7 +50,7 @@ fn main() {
         println!("Starting a new build simulation");
         let mut actions =
             build::JobActions::new(&cfg.nix.system, &cfg.runner.identity, &job, &mut receiver);
-        actions.log_started();
+        actions.log_started(vec![], vec![]);
 
         for i in 1..51 {
             actions.log_line(&format!("Bogus message  #{:?}/50", i));

ofborg/src/message/buildlogmsg.rs

@@ -13,4 +13,6 @@ pub struct BuildLogStart {
     pub system: String,
     pub identity: String,
     pub attempt_id: String,
+    pub attempted_attrs: Option<Vec<String>>,
+    pub skipped_attrs: Option<Vec<String>>,
 }

ofborg/src/message/buildresult.rs

@@ -7,5 +7,7 @@ pub struct BuildResult {
     pub system: String,
     pub output: Vec<String>,
     pub attempt_id: Option<String>,
-    pub success: bool,
+    pub success: Option<bool>,
+    pub skipped_attrs: Option<Vec<String>>,
+    pub attempted_attrs: Option<Vec<String>>,
 }

ofborg/src/nix.rs

@@ -97,6 +97,50 @@ impl Nix {
         return n;
     }
 
+    pub fn safely_partition_instantiable_attrs(
+        &self,
+        nixpkgs: &Path,
+        file: &str,
+        attrs: Vec<String>,
+    ) -> (Vec<String>, Vec<String>) {
+        attrs
+            .into_iter()
+            .partition(|attr| {
+                self.safely_instantiate_attrs(
+                    nixpkgs,
+                    file,
+                    vec![attr.clone()]
+                ).is_ok()
+            })
+    }
+
+    pub fn safely_instantiate_attrs(
+        &self,
+        nixpkgs: &Path,
+        file: &str,
+        attrs: Vec<String>,
+    ) -> Result<File, File> {
+        let cmd = self.safely_instantiate_attrs_cmd(nixpkgs, file, attrs);
+
+        return self.run(cmd, true);
+    }
+
+    pub fn safely_instantiate_attrs_cmd(
+        &self,
+        nixpkgs: &Path,
+        file: &str,
+        attrs: Vec<String>,
+    ) -> Command {
+        let mut attrargs: Vec<String> = Vec::with_capacity(3 + (attrs.len() * 2));
+        attrargs.push(file.to_owned());
+        for attr in attrs {
+            attrargs.push(String::from("-A"));
+            attrargs.push(attr);
+        }
+
+        return self.safe_command(Operation::Instantiate, nixpkgs, attrargs);
+    }
+
     pub fn safely_build_attrs(
         &self,
         nixpkgs: &Path,
@@ -235,6 +279,12 @@ mod tests {
         return cwd;
     }
 
+    fn individual_eval_path() -> PathBuf {
+        let mut cwd = env::current_dir().unwrap();
+        cwd.push(Path::new("./test-srcs/eval-mixed-failure"));
+        return cwd;
+    }
+
     #[derive(Debug)]
     enum Expect {
         Pass,
@@ -509,6 +559,63 @@ mod tests {
         );
     }
 
+    #[test]
+    fn partition_instantiable_attributes() {
+        let nix = nix();
+
+        let ret: (Vec<String>, Vec<String>) = nix.safely_partition_instantiable_attrs(
+            individual_eval_path().as_path(),
+            "default.nix",
+            vec![
+                String::from("fails-instantiation"),
+                String::from("passes-instantiation"),
+                String::from("missing-attr"),
+            ],
+        );
+
+        assert_eq!(ret.0, vec!["passes-instantiation"]);
+        assert_eq!(ret.1, vec!["fails-instantiation", "missing-attr"]);
+    }
+
+    #[test]
+    fn safely_instantiate_attrs_failure() {
+        let nix = nix();
+
+        let ret: Result<File, File> = nix.safely_instantiate_attrs(
+            individual_eval_path().as_path(),
+            "default.nix",
+            vec![String::from("fails-instantiation")],
+        );
+
+        assert_run(
+            ret,
+            Expect::Fail,
+            vec![
+                "You just can't",
+                "assertion failed",
+            ],
+        );
+    }
+
+    #[test]
+    fn safely_instantiate_attrs_success() {
+        let nix = nix();
+
+        let ret: Result<File, File> = nix.safely_instantiate_attrs(
+            individual_eval_path().as_path(),
+            "default.nix",
+            vec![String::from("passes-instantiation")],
+        );
+
+        assert_run(
+            ret,
+            Expect::Pass,
+            vec![
+                "-passes-instantiation.drv"
+            ],
+        );
+    }
+
     #[test]
     fn strict_sandboxing() {
         let ret: Result<File, File> = nix().safely_build_attrs(

ofborg/src/tasks/build.rs

@@ -116,7 +116,9 @@ impl<'a, 'b> JobActions<'a, 'b> {
             system: self.system.clone(),
             output: vec![String::from("Merge failed")],
             attempt_id: Some(self.attempt_id.clone()),
-            success: false,
+            attempted_attrs: None,
+            skipped_attrs: None,
+            success: Some(false),
         };
 
         let result_exchange = self.result_exchange.clone();
@@ -131,11 +133,13 @@ impl<'a, 'b> JobActions<'a, 'b> {
         self.tell(worker::Action::Ack);
     }
 
-    pub fn log_started(&mut self) {
+    pub fn log_started(&mut self, can_build: Vec<String>, cannot_build: Vec<String>) {
         let msg = buildlogmsg::BuildLogStart {
             identity: self.identity.clone(),
             system: self.system.clone(),
             attempt_id: self.attempt_id.clone(),
+            attempted_attrs: Some(can_build),
+            skipped_attrs: Some(cannot_build),
         };
 
         let log_exchange = self.log_exchange.clone();
@@ -169,20 +173,50 @@ impl<'a, 'b> JobActions<'a, 'b> {
         ));
     }
 
-    pub fn build_finished(&mut self, success: bool, lines: Vec<String>) {
+    pub fn build_not_attempted(&mut self, not_attempted_attrs: Vec<String>,
+
+    ) {
+        let msg = buildresult::BuildResult {
+            repo: self.job.repo.clone(),
+            pr: self.job.pr.clone(),
+            system: self.system.clone(),
+            output: vec![],
+            attempt_id: Some(self.attempt_id.clone()),
+            skipped_attrs: Some(not_attempted_attrs),
+            attempted_attrs: None,
+            success: None,
+        };
+
+        let result_exchange = self.result_exchange.clone();
+        let result_routing_key = self.result_routing_key.clone();
+
+        self.tell(worker::publish_serde_action(
+            result_exchange,
+            result_routing_key,
+            &msg,
+        ));
+        self.tell(worker::Action::Ack);
+    }
+
+    pub fn build_finished(&mut self, success: bool, lines: Vec<String>,
+                          attempted_attrs: Vec<String>,
+                          not_attempted_attrs: Vec<String>,
+
+    ) {
         let msg = buildresult::BuildResult {
             repo: self.job.repo.clone(),
             pr: self.job.pr.clone(),
             system: self.system.clone(),
             output: lines,
             attempt_id: Some(self.attempt_id.clone()),
-            success: success,
+            success: Some(success),
+            attempted_attrs: Some(attempted_attrs),
+            skipped_attrs: Some(not_attempted_attrs),
         };
 
         let result_exchange = self.result_exchange.clone();
         let result_routing_key = self.result_routing_key.clone();
 
-
         self.tell(worker::publish_serde_action(
             result_exchange,
             result_routing_key,
@@ -266,16 +300,29 @@ impl notifyworker::SimpleNotifyWorker for BuildWorker {
             return;
         }
 
-        println!("Got path: {:?}, building", refpath);
+        println!("Got path: {:?}, determining which ones we can build ", refpath);
-
+        let (can_build, cannot_build) = self.nix.safely_partition_instantiable_attrs(
-
-        let cmd = self.nix.safely_build_attrs_cmd(
             refpath.as_ref(),
             buildfile,
             job.attrs.clone(),
         );
 
-        actions.log_started();
+        println!("Can build: {}, Cannot build: {}",
+                 can_build.join(", "),
+                 cannot_build.join(", "));
+
+        if can_build.len() == 0 {
+            actions.build_not_attempted(cannot_build);
+            return;
+        }
+
+        let cmd = self.nix.safely_build_attrs_cmd(
+            refpath.as_ref(),
+            buildfile,
+            can_build.clone(),
+        );
+
+        actions.log_started(can_build.clone(), cannot_build.clone());
         let acmd = AsyncCmd::new(cmd);
         let mut spawned = acmd.spawn();
 
@@ -311,7 +358,7 @@ impl notifyworker::SimpleNotifyWorker for BuildWorker {
 
         let last10lines: Vec<String> = snippet_log.into_iter().collect::<Vec<String>>();
 
-        actions.build_finished(success, last10lines.clone());
+        actions.build_finished(success, last10lines.clone(), can_build, cannot_build);
         println!("Done!");
     }
 }
@@ -396,7 +443,6 @@ mod tests {
         let head_sha = make_pr_repo(&bare_repo.path(), &co_repo.path());
         let worker = make_worker(&p.path());
 
-
         let job = buildjob::BuildJob {
             attrs: vec!["success".to_owned()],
             pr: Pr {
@@ -433,4 +479,45 @@ mod tests {
         assert_contains_job(&mut actions, "success\":true");
         assert_eq!(actions.next(), Some(worker::Action::Ack));
     }
+
+
+    #[test]
+    pub fn test_all_jobs_skipped() {
+        let p = TestScratch::new_dir("no-attempt");
+        let bare_repo = TestScratch::new_dir("no-attempt-bare");
+        let co_repo = TestScratch::new_dir("no-attempt-co");
+
+        let head_sha = make_pr_repo(&bare_repo.path(), &co_repo.path());
+        let worker = make_worker(&p.path());
+
+        let job = buildjob::BuildJob {
+            attrs: vec!["not-real".to_owned()],
+            pr: Pr {
+                head_sha: head_sha,
+                number: 1,
+                target_branch: Some("master".to_owned()),
+            },
+            repo: Repo {
+                clone_url: bare_repo.path().to_str().unwrap().to_owned(),
+                full_name: "test-git".to_owned(),
+                name: "nixos".to_owned(),
+                owner: "ofborg-test".to_owned(),
+            },
+            subset: None,
+            logs: Some((
+                Some(String::from("logs")),
+                Some(String::from("build.log")),
+            )),
+            statusreport: Some((Some(String::from("build-results")), None)),
+        };
+
+        let mut dummyreceiver = notifyworker::DummyNotificationReceiver::new();
+
+        worker.consumer(&job, &mut dummyreceiver);
+
+        println!("Total actions: {:?}", dummyreceiver.actions.len());
+        let mut actions = dummyreceiver.actions.into_iter();
+        assert_contains_job(&mut actions, "skipped_attrs\":[\"not-real");
+        assert_eq!(actions.next(), Some(worker::Action::Ack));
+    }
 }

ofborg/src/tasks/githubcommentposter.rs

@@ -74,13 +74,9 @@ impl worker::SimpleWorker for GitHubCommentPoster {
 fn result_to_comment(result: &BuildResult) -> String {
     let mut reply: Vec<String> = vec![];
 
-    reply.push(format!(
+    let log_link = match result.success {
-        "{} on {} [(full log)](https://logs.nix.ci/?key={}/{}.{}&attempt_id={})",
+        Some(_) => format!(
-        (match result.success {
+            " [(full log)](https://logs.nix.ci/?key={}/{}.{}&attempt_id={})",
-             true => "Success",
-             false => "Failure",
-         }),
-        result.system,
             &result.repo.owner.to_lowercase(),
             &result.repo.name.to_lowercase(),
             result.pr.number,
@@ -88,20 +84,71 @@ fn result_to_comment(result: &BuildResult) -> String {
                 Some(ref attempt_id) => &attempt_id,
                 None => "none",
             })
+        ),
+        None => "".to_owned()
+    };
+
+    reply.push(format!(
+        "{} on {}{}",
+        (match result.success {
+            Some(true) => "Success",
+            Some(false) => "Failure",
+            None => "No attempt",
+         }),
+        result.system,
+        log_link
     ));
     reply.push("".to_owned());
+
+    if let Some(ref attempted) = result.attempted_attrs {
+        reply.extend(list_segment("Attempted", attempted.clone()));
+    }
+
+    if let Some(ref skipped) = result.skipped_attrs {
+        reply.extend(list_segment(
+            &format!(
+                "The following builds were skipped because they don't evaluate on {}",
+                result.system
+            ),
+            skipped.clone()));
+    }
+
+    if result.output.len() > 0 {
+        reply.extend(partial_log_segment(&result.output));
+    } else {
+        reply.push("No log is available.".to_owned());
+        reply.push("".to_owned());
+    }
+
+    reply.join("\n")
+}
+
+fn list_segment(name: &str, things: Vec<String>) -> Vec<String> {
+    let mut reply: Vec<String> = vec![];
+
+    if things.len() > 0 {
+        reply.push(format!("{}: {}", name, things.join(", ")));
+        reply.push("".to_owned());
+    }
+
+    return reply;
+}
+
+fn partial_log_segment(output: &Vec<String>) -> Vec<String> {
+    let mut reply: Vec<String> = vec![];
+
     reply.push(
         "<details><summary>Partial log (click to expand)</summary><p>".to_owned(),
     );
     reply.push("".to_owned());
     reply.push("```".to_owned());
-    reply.extend(result.output.clone());
+    reply.extend(output.clone());
     reply.push("```".to_owned());
     reply.push("</p></details>".to_owned());
     reply.push("".to_owned());
     reply.push("".to_owned());
 
-    reply.join("\n")
+    return reply;
 }
 
 #[cfg(test)]
@@ -137,13 +184,19 @@ mod tests {
             ],
             attempt_id: Some("neatattemptid".to_owned()),
             system: "x86_64-linux".to_owned(),
-            success: true,
+            attempted_attrs: Some(vec!["foo".to_owned()]),
+            skipped_attrs: Some(vec!["bar".to_owned()]),
+            success: Some(true),
         };
 
         assert_eq!(
             &result_to_comment(&result),
             "Success on x86_64-linux [(full log)](https://logs.nix.ci/?key=nixos/nixpkgs.2345&attempt_id=neatattemptid)
 
+Attempted: foo
+
+The following builds were skipped because they don't evaluate on x86_64-linux: bar
+
 <details><summary>Partial log (click to expand)</summary><p>
 
 ```
@@ -192,7 +245,126 @@ patching script interpreter paths in /nix/store/pcja75y9isdvgz5i00pkrpif9rxzxc29
             ],
             attempt_id: Some("neatattemptid".to_owned()),
             system: "x86_64-linux".to_owned(),
-            success: false,
+            attempted_attrs: Some(vec!["foo".to_owned()]),
+            skipped_attrs: None,
+            success: Some(false),
+        };
+
+        assert_eq!(
+            &result_to_comment(&result),
+            "Failure on x86_64-linux [(full log)](https://logs.nix.ci/?key=nixos/nixpkgs.2345&attempt_id=neatattemptid)
+
+Attempted: foo
+
+<details><summary>Partial log (click to expand)</summary><p>
+
+```
+make[2]: Entering directory '/private/tmp/nix-build-gdb-8.1.drv-0/gdb-8.1/readline'
+make[2]: Nothing to be done for 'install'.
+make[2]: Leaving directory '/private/tmp/nix-build-gdb-8.1.drv-0/gdb-8.1/readline'
+make[1]: Nothing to be done for 'install-target'.
+make[1]: Leaving directory '/private/tmp/nix-build-gdb-8.1.drv-0/gdb-8.1'
+removed '/nix/store/pcja75y9isdvgz5i00pkrpif9rxzxc29-gdb-8.1/share/info/bfd.info'
+post-installation fixup
+strip is /nix/store/5a88zk3jgimdmzg8rfhvm93kxib3njf9-cctools-binutils-darwin/bin/strip
+patching script interpreter paths in /nix/store/pcja75y9isdvgz5i00pkrpif9rxzxc29-gdb-8.1
+/nix/store/pcja75y9isdvgz5i00pkrpif9rxzxc29-gdb-8.1
+```
+</p></details>
+
+"
+        );
+    }
+
+
+    #[test]
+    pub fn test_passing_build_unspecified_attributes() {
+        let result = BuildResult {
+            repo: Repo {
+                clone_url: "https://github.com/nixos/nixpkgs.git".to_owned(),
+                full_name: "NixOS/nixpkgs".to_owned(),
+                owner: "NixOS".to_owned(),
+                name: "nixpkgs".to_owned(),
+            },
+            pr: Pr {
+                head_sha: "abc123".to_owned(),
+                number: 2345,
+                target_branch: Some("master".to_owned()),
+            },
+            output: vec![
+                "make[2]: Entering directory '/private/tmp/nix-build-gdb-8.1.drv-0/gdb-8.1/readline'".to_owned(),
+                "make[2]: Nothing to be done for 'install'.".to_owned(),
+                "make[2]: Leaving directory '/private/tmp/nix-build-gdb-8.1.drv-0/gdb-8.1/readline'".to_owned(),
+                "make[1]: Nothing to be done for 'install-target'.".to_owned(),
+                "make[1]: Leaving directory '/private/tmp/nix-build-gdb-8.1.drv-0/gdb-8.1'".to_owned(),
+                "removed '/nix/store/pcja75y9isdvgz5i00pkrpif9rxzxc29-gdb-8.1/share/info/bfd.info'".to_owned(),
+                "post-installation fixup".to_owned(),
+                "strip is /nix/store/5a88zk3jgimdmzg8rfhvm93kxib3njf9-cctools-binutils-darwin/bin/strip".to_owned(),
+                "patching script interpreter paths in /nix/store/pcja75y9isdvgz5i00pkrpif9rxzxc29-gdb-8.1".to_owned(),
+                "/nix/store/pcja75y9isdvgz5i00pkrpif9rxzxc29-gdb-8.1".to_owned(),
+            ],
+            attempt_id: Some("neatattemptid".to_owned()),
+            system: "x86_64-linux".to_owned(),
+            attempted_attrs: None,
+            skipped_attrs: None,
+            success: Some(true),
+        };
+
+        assert_eq!(
+            &result_to_comment(&result),
+            "Success on x86_64-linux [(full log)](https://logs.nix.ci/?key=nixos/nixpkgs.2345&attempt_id=neatattemptid)
+
+<details><summary>Partial log (click to expand)</summary><p>
+
+```
+make[2]: Entering directory '/private/tmp/nix-build-gdb-8.1.drv-0/gdb-8.1/readline'
+make[2]: Nothing to be done for 'install'.
+make[2]: Leaving directory '/private/tmp/nix-build-gdb-8.1.drv-0/gdb-8.1/readline'
+make[1]: Nothing to be done for 'install-target'.
+make[1]: Leaving directory '/private/tmp/nix-build-gdb-8.1.drv-0/gdb-8.1'
+removed '/nix/store/pcja75y9isdvgz5i00pkrpif9rxzxc29-gdb-8.1/share/info/bfd.info'
+post-installation fixup
+strip is /nix/store/5a88zk3jgimdmzg8rfhvm93kxib3njf9-cctools-binutils-darwin/bin/strip
+patching script interpreter paths in /nix/store/pcja75y9isdvgz5i00pkrpif9rxzxc29-gdb-8.1
+/nix/store/pcja75y9isdvgz5i00pkrpif9rxzxc29-gdb-8.1
+```
+</p></details>
+
+"
+        );
+    }
+
+    #[test]
+    pub fn test_failing_build_unspecified_attributes() {
+        let result = BuildResult {
+            repo: Repo {
+                clone_url: "https://github.com/nixos/nixpkgs.git".to_owned(),
+                full_name: "NixOS/nixpkgs".to_owned(),
+                owner: "NixOS".to_owned(),
+                name: "nixpkgs".to_owned(),
+            },
+            pr: Pr {
+                head_sha: "abc123".to_owned(),
+                number: 2345,
+                target_branch: Some("master".to_owned()),
+            },
+            output: vec![
+                "make[2]: Entering directory '/private/tmp/nix-build-gdb-8.1.drv-0/gdb-8.1/readline'".to_owned(),
+                "make[2]: Nothing to be done for 'install'.".to_owned(),
+                "make[2]: Leaving directory '/private/tmp/nix-build-gdb-8.1.drv-0/gdb-8.1/readline'".to_owned(),
+                "make[1]: Nothing to be done for 'install-target'.".to_owned(),
+                "make[1]: Leaving directory '/private/tmp/nix-build-gdb-8.1.drv-0/gdb-8.1'".to_owned(),
+                "removed '/nix/store/pcja75y9isdvgz5i00pkrpif9rxzxc29-gdb-8.1/share/info/bfd.info'".to_owned(),
+                "post-installation fixup".to_owned(),
+                "strip is /nix/store/5a88zk3jgimdmzg8rfhvm93kxib3njf9-cctools-binutils-darwin/bin/strip".to_owned(),
+                "patching script interpreter paths in /nix/store/pcja75y9isdvgz5i00pkrpif9rxzxc29-gdb-8.1".to_owned(),
+                "/nix/store/pcja75y9isdvgz5i00pkrpif9rxzxc29-gdb-8.1".to_owned(),
+            ],
+            attempt_id: Some("neatattemptid".to_owned()),
+            system: "x86_64-linux".to_owned(),
+            attempted_attrs: None,
+            skipped_attrs: None,
+            success: Some(false),
         };
 
         assert_eq!(
@@ -247,7 +419,9 @@ patching script interpreter paths in /nix/store/pcja75y9isdvgz5i00pkrpif9rxzxc29
             ],
             attempt_id: None,
             system: "x86_64-linux".to_owned(),
-            success: false,
+            attempted_attrs: None,
+            skipped_attrs: None,
+            success: Some(false),
         };
 
         assert_eq!(
@@ -270,6 +444,39 @@ patching script interpreter paths in /nix/store/pcja75y9isdvgz5i00pkrpif9rxzxc29
 ```
 </p></details>
 
+"
+        );
+    }
+
+    #[test]
+    pub fn test_no_attempt() {
+        let result = BuildResult {
+            repo: Repo {
+                clone_url: "https://github.com/nixos/nixpkgs.git".to_owned(),
+                full_name: "NixOS/nixpkgs".to_owned(),
+                owner: "NixOS".to_owned(),
+                name: "nixpkgs".to_owned(),
+            },
+            pr: Pr {
+                head_sha: "abc123".to_owned(),
+                number: 2345,
+                target_branch: Some("master".to_owned()),
+            },
+            output: vec![],
+            attempt_id: Some("foo".to_owned()),
+            system: "x86_64-linux".to_owned(),
+            attempted_attrs: None,
+            skipped_attrs: Some(vec!["not-attempted".to_owned()]),
+            success: None,
+        };
+
+        assert_eq!(
+            &result_to_comment(&result),
+            "No attempt on x86_64-linux
+
+The following builds were skipped because they don't evaluate on x86_64-linux: not-attempted
+
+No log is available.
 "
         );
     }

ofborg/src/tasks/log_message_collector.rs

@@ -345,6 +345,8 @@ mod tests {
                                                attempt_id: String::from("my-attempt-id"),
                                                identity: String::from("my-identity"),
                                                system: String::from("foobar-x8664"),
+                                               attempted_attrs: Some(vec!["foo".to_owned()]),
+                                               skipped_attrs: Some(vec!["bar".to_owned()]),
                                            })
                                        }
                        )
@@ -369,7 +371,7 @@ mod tests {
         let mut s = String::new();
         pr.push("routing-key-foo/attempt-id-foo.metadata.json");
         File::open(pr).unwrap().read_to_string(&mut s).unwrap();
-        assert_eq!(&s, "{\"system\":\"foobar-x8664\",\"identity\":\"my-identity\",\"attempt_id\":\"my-attempt-id\"}");
+        assert_eq!(&s, "{\"system\":\"foobar-x8664\",\"identity\":\"my-identity\",\"attempt_id\":\"my-attempt-id\",\"attempted_attrs\":[\"foo\"],\"skipped_attrs\":[\"bar\"]}");
 
 
         let mut pr = p.path();