Closed Bug 1529296 Opened 1 year ago Closed 10 months ago

Add build::build_info() -> BuildInfo function

Categories

(Testing :: geckodriver, enhancement, P1)

Version 3
enhancement

Tracking

(firefox71 fixed)

RESOLVED FIXED
mozilla71
Tracking Status
firefox71 --- fixed

People

(Reporter: ato, Assigned: natquaylenelson)

References

Details

(Keywords: good-first-bug)

Attachments

(1 file)

For no other good reason than making it a bit more Rust-esque, we
should add a build_info() -> BuildInfo function to
testing/geckodriver/build.rs. This also requires a couple of
changes throughout geckodriver, which you can test with ./mach test testing/geckodriver.

Read more about working on geckodriver at
https://firefox-source-docs.mozilla.org/testing/geckodriver/.

Keywords: good-first-bug

Good evening! I'm interested in contributing to this project via Outreachy and using/expanding upon my Rust knowledge. Can I be assigned this issue?

In the meantime I'll work on checking out the code and getting a working dev environment together so I can take on this or anything else.

Thanks!

You may find these links useful to get started:

We also have #interop on irc.mozilla.org in case you have any questions.

Assignee: nobody → natquaylenelson
Status: NEW → ASSIGNED

I'm stuck trying to run the geckodriver build with ./mach build testing/geckodriver. The cargo build errors with the following output: Pastebin

After cloning the repo through bootstrap.py, I also ran ./mach bootstrap. I don't know if those are two separate things but I wanted to be safe.

I'm going to make sure bzip is installed on my system, but that's my only guess for how to fix this.

Thanks for reporting this.

(In reply to Nat Quayle Nelson from comment #3)

I'm going to make sure bzip is installed on my system, but that's
my only guess for how to fix this.

Yes, you need the bzip2 headers, found on Debian in libbz2-dev.
I’ve filed https://bugzilla.mozilla.org/show_bug.cgi?id=1531483 to
have this installed by ./mach bootstrap.

(In reply to Andreas Tolfsen ⦗:ato⦘ from comment #4)

Yes, you need the bzip2 headers, found on Debian in libbz2-dev.
I’ve filed https://bugzilla.mozilla.org/show_bug.cgi?id=1531483 to
have this installed by ./mach bootstrap.

That bug was marked as invalid. What else do we have to fix to get mach build testing/geckodriver working?

For now I assume running cargo build in testing/geckodriver will be fine and should work. I think I experienced the same thing not that long ago.

Flags: needinfo?(ato)

I already resolved this with Nat on IRC.

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+1] (away 02/28 - 03/03) from comment #5)

(In reply to Andreas Tolfsen ⦗:ato⦘ from comment #4)

Yes, you need the bzip2 headers, found
on Debian in libbz2-dev. I’ve filed
https://bugzilla.mozilla.org/show_bug.cgi?id=1531483 to have
this installed by ./mach bootstrap.

That bug was marked as invalid. What else do we have to fix to get
mach build testing/geckodriver working?

./mach configure

For now I assume running cargo build in testing/geckodriver
will be fine and should work. I think I experienced the same thing
not that long ago.

Yes, because that fetches the build dependencies instead of using
those from third_party.

Flags: needinfo?(ato)

I could use some clarification on the desired change for the build_info issue. Is the goal essentially to replace the existing get_build_info() with a function that needs no arguments and returns a value, not a Box?

If that's the case, I'm wondering what changes to geckodriver would be necessary. It seems like build.rs would be the only file to change.

I realise the text in comment #0 is incorrect: it references
testing/geckodriver/build.rs but should instead say
testing/geckodriver/src/build.rs. The link, however, is correct.
The difference between these is that the first is used at build
time to produce some static content later used by the module
build. This bug is about adding a new function to the module,
returning the BuildInfo struct.

Sorry for the confusion, but does that help?

Priority: -- → P1

Hi, is this issue resolved yet? I would like to complete it if possible.

It looks like Nat forgot to request review on this patch.
But the patch looks good, so I pick it up and make sure it lands.

Flags: needinfo?(ato)
Blocks: 1573798
Flags: needinfo?(ato)
Pushed by ato@sny.no:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d8ea766a18e5
geckodriver: add build_info() public API; r=ato
Status: ASSIGNED → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla71
You need to log in before you can comment on or make changes to this bug.