Henrik Skupin [:whimboo][⌚️UTC+1] Reporter
	Description • 6 years ago

Today I tried to compile geckodriver but it fails with an error in mozrunner. As it looks like the changes from bug 1464995 are conflicting. Not sure why this isn't visible in CI.

   Compiling mozrunner v0.6.1 (file:///Volumes/data/code/gecko/testing/mozbase/rust/mozrunner)
error[E0599]: no method named `to_path_buf` found for type `&str` in the current scope
   --> testing/mozbase/rust/mozrunner/src/runner.rs:382:42
    |
382 |                 (_, false) => trial_path.to_path_buf(),
    |                                          ^^^^^^^^^^^

error[E0308]: mismatched types
   --> testing/mozbase/rust/mozrunner/src/runner.rs:385:29
    |
385 |                 return Some(path);
    |                             ^^^^
    |                             |
    |                             expected struct `std::path::PathBuf`, found reference
    |                             help: try using a conversion method: `path.to_path_buf()`
    |
    = note: expected type `std::path::PathBuf`
               found type `&std::path::PathBuf`

	Andreas Tolfsen ❲:ato❳ Assignee
	Comment 1 • 6 years ago

It doesn’t show on try because we don’t compile geckodriver on
macOS, and this is in a #[cfg(target_os = "macos")] block.

	Henrik Skupin [:whimboo][⌚️UTC+1] Reporter
	Comment 2 • 6 years ago

So should we revert this particular change which even for styling only.

	Andreas Tolfsen ❲:ato❳ Assignee
	Comment 3 • 6 years ago

Reverting seems excessive.  It’s easy enough to fix.

	Henrik Skupin [:whimboo][⌚️UTC+1] Reporter
	Comment 5 • 6 years ago

(In reply to Andreas Tolfsen 「:ato」 from comment #3)
> Reverting seems excessive.  It’s easy enough to fix.

Hm? The attached patch exactly reverts the changes to those lines. So maybe you missunderstood me. My intention was not to revert the whole patch...

	Henrik Skupin [:whimboo][⌚️UTC+1] Reporter
	Comment 6 • 6 years ago
mozreview-review

Comment on attachment 8984418 [details]
Bug 1467728 - Fix mozrunner build on macOS.

https://reviewboard.mozilla.org/r/250254/#review256580

	Andreas Tolfsen ❲:ato❳ Assignee
	Comment 7 • 6 years ago

upin (:whimboo) from comment #5)

> (In reply to Andreas Tolfsen 「:ato」 from comment #3)  
> 
> > Reverting seems excessive.  It’s easy enough to fix.
> 
> Hm? The attached patch exactly reverts the changes to those
> lines. So maybe you missunderstood me. My intention was not to
> revert the whole patch...

Right, I thought you meant (1) back out original patches, (2)
re-apply patches without bug.

Thanks for the review.

	Pulsebot
	Comment 8 • 6 years ago

Pushed by atolfsen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/dd482a0086c4
Fix mozrunner build on macOS. r=whimboo

	Bogdan Tara[:bogdan_tara | bogdant]
	Comment 9 • 6 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/dd482a0086c4