Closed
Bug 1368265
Opened 7 years ago
Closed 7 years ago
Import and build webdriver crate
Categories
(Testing :: geckodriver, defect, P2)
Tracking
(firefox57 fixed)
RESOLVED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox57 | --- | fixed |
People
(Reporter: ato, Assigned: ato)
References
(Blocks 1 open bug)
Details
Attachments
(6 files, 1 obsolete file)
59 bytes,
text/x-review-board-request
|
automatedtester
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
automatedtester
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
automatedtester
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
automatedtester
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
automatedtester
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
automatedtester
:
review+
|
Details |
We want to import the git history and build the webdriver Rust crate in mozilla-central: https://github.com/mozilla/webdriver-rust We will then tombstone the GitHub project repository and (maybe later?) hook it up as a dependency to testing/geckodriver.
Assignee | ||
Updated•7 years ago
|
Priority: -- → P2
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → ato
Status: NEW → ASSIGNED
Comment 1•7 years ago
|
||
Hm, what will be the target directory for the source? I feel that we should not cluttering /testing/ that much, given that there are also other rust related repos still in question to be imported too.
Assignee | ||
Comment 2•7 years ago
|
||
(In reply to Henrik Skupin (:whimboo) from comment #1) > Hm, what will be the target directory for the source? I feel that we should > not cluttering /testing/ that much, given that there are also other rust > related repos still in question to be imported too. I intend to put it under testing/geckodriver and change testing/geckodriver/Cargo.toml’s webdriver entry to point to ../geckodriver. I don’t know what you mean about “cluttering” the testing directory. Can you expand? What is a better directory?
Comment 3•7 years ago
|
||
With cluttering I meant that we might not want to put every rust repository under /testing. So I assume you meant to have something like testing/geckodriver/webdriver? If that is the case you might want to think about using a workspace in your Cargo.toml. Also because I assume that geckodriver is the only consumer of webdriver-rust?
Assignee | ||
Comment 4•7 years ago
|
||
(In reply to Henrik Skupin (:whimboo) from comment #3) > With cluttering I meant that we might not want to put every rust > repository under /testing. Irregardless what programming language is used, code should live where it is most appropriate. If there are good reasons that speak for putting code in a sub-directory, then we should listen to those. > So I assume you meant to have something like > testing/geckodriver/webdriver? If that is the case you might want > to think about using a workspace in your Cargo.toml. Also because I > assume that geckodriver is the only consumer of webdriver-rust? The webdriver crate is a separate library also used in Servo. Whilst it is theoretically possible to put it in testing/geckodriver/webdriver, I don’t think a cargo workspace has any practical benefit except inflating the number of directory levels you need to traverse to access the code?
Comment 5•7 years ago
|
||
(In reply to Andreas Tolfsen ‹:ato› from comment #4) > The webdriver crate is a separate library also used in Servo. Whilst it > is theoretically possible to put it in testing/geckodriver/webdriver, > I don’t think a cargo workspace has any practical benefit except > inflating the number of directory levels you need to traverse to access > the code? It might make it easier for our build config so only one project has to be specified, and the other is just build in tandem. Both would also use the exact same dependencies, as specified in the top-level Cargo.lock. Also aren't both closely related? If it doesn't make sense feel free to drop my comments.
Assignee | ||
Comment 6•7 years ago
|
||
(In reply to Henrik Skupin (:whimboo) from comment #5) > (In reply to Andreas Tolfsen ‹:ato› from comment #4) > > The webdriver crate is a separate library also used in Servo. Whilst it > > is theoretically possible to put it in testing/geckodriver/webdriver, > > I don’t think a cargo workspace has any practical benefit except > > inflating the number of directory levels you need to traverse to access > > the code? > > It might make it easier for our build config so only one project has to be > specified, and the other is just build in tandem. Both would also use the > exact same dependencies, as specified in the top-level Cargo.lock. Also > aren't both closely related? As far as I understand, no special changes need to be made to the build config. We just need to tell geckodriver to pick up the webdriver dependency from correct relative path. From the point of the build system, the target is geckodriver, and it’s up to cargo where it finds the dependencies. Whether it finds the dependencies in /third_party/rust as it does now, or from somewhere else, is irrelevant.
Comment 7•7 years ago
|
||
(In reply to Andreas Tolfsen ‹:ato› from comment #0) > it up as a dependency to testing/geckodriver. I filed bug 1369086 to get this component created.
Updated•7 years ago
|
Component: Marionette → geckodriver
Assignee | ||
Comment 8•7 years ago
|
||
I’ve now linearised the git repository history from https://github.com/mozilla/webdriver-rust.git and overlaid it onto mozilla-inbound like we did for testing/geckodriver, based on gps’ instructions in https://bugzilla.mozilla.org/show_bug.cgi?id=1340637#c58. The result of this is now available at https://hg.mozilla.org/users/atolfsen_mozilla.com/gecko/. Because I’m unable to push this to inbound myself (“Rev […] needs "Bug N" or "No bug" in the commit message” error), I need help to put these commits into upstream. Could you do the honours or let me know how to bypass that restriction? The graphically annotated log can be viewed at https://gist.githubusercontent.com/andreastt/a08f0829986aed65163ea10476b1207b/raw/66069e8a58a410d745006d71b0bc159f2e783da0/gistfile1.txt
Flags: needinfo?(gps)
Assignee | ||
Comment 9•7 years ago
|
||
For posterity I’ve documented the steps to linearise and overlay a
git repository into mozilla-central:
> % cd /home/ato/src
> % git clone hg::https://hg.mozilla.org/hgcustom/version-control-tools
> % cat <<EOF >>/home/ato/.hgrc
> [extensions]
> overlay = ~/src/version-control-tools/hgext/overlay
> EOF
> % cd version-control-tools
> % ./create-environment vcssync
> % source venv/vcssync/bin/activate
> % cd /tmp
> % linearize-git-to-hg --sumary-prefix 'webdriver:' \
> --source-repo-key Source-Repository \
> --source-revision-key Source-Revision \
> --use-p2-author \
> --copy-similarity 75 \
> --find-copies-harder \
> https://github.com/mozilla/webdriver-rust \
> master \
> /tmp/webdriver-rust-git \
> /tmp/webdriver-rust-hg
> % hg clone https://hg.mozilla.org/integration/mozilla-inbound
> % hg --cwd webdriver-rust-hg serve &
> [1] 28156
> % cd mozilla-inbound
> % hg overlay --into testing/webdriver http://tristan.corp.lon2.mozilla.com:8000
> % hg push -f ssh://hg.mozilla.org/users/atolfsen_mozilla.com/gecko
Comment 10•7 years ago
|
||
You can add "IGNORE BAD COMMIT MESSAGES" to the commit that is pushed. Alternatively, we could merge this repo into mozilla-central by introducing a new root commit in the repo. That is what we did with Servo. The latter requires us to add the root commit's SHA-1 to the server config on hg.mo.
Flags: needinfo?(gps)
Assignee | ||
Comment 11•7 years ago
|
||
(In reply to Gregory Szorc [:gps] from comment #10) > You can add "IGNORE BAD COMMIT MESSAGES" to the commit that > is pushed. Alternatively, we could merge this repo into > mozilla-central by introducing a new root commit in the repo. That > is what we did with Servo. The latter requires us to add the root > commit's SHA-1 to the server config on hg.mo. Thanks for the instructions! Because whimboo had landed a few commits to the git repository after I did the conversion process, I re-did the steps above with the slight alteration s/Source-Repository/Source-Repo/ as source repo key. Pushlog: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?changeset=a12424febc5e41c5d52d77fb01f6b86f0150f6cd Treeherder: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=a12424febc5e41c5d52d77fb01f6b86f0150f6cd
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 19•7 years ago
|
||
mozreview-review |
Comment on attachment 8904042 [details] Bug 1368265 - Use in-tree testing/webdriver. https://reviewboard.mozilla.org/r/175772/#review180986
Attachment #8904042 -
Flags: review?(dburns) → review+
Comment 20•7 years ago
|
||
mozreview-review |
Comment on attachment 8904043 [details] Bug 1368265 - Remove third_party/rust/webdriver. https://reviewboard.mozilla.org/r/175774/#review180988
Attachment #8904043 -
Flags: review?(dburns) → review+
Comment 21•7 years ago
|
||
mozreview-review |
Comment on attachment 8904044 [details] Bug 1368265 - Update geckodriver and webdriver Cargo.tomls. https://reviewboard.mozilla.org/r/175776/#review180990
Attachment #8904044 -
Flags: review?(dburns) → review+
Comment 22•7 years ago
|
||
mozreview-review |
Comment on attachment 8904045 [details] Bug 1368265 - Provide a webdriver README. https://reviewboard.mozilla.org/r/175778/#review180992 ::: testing/webdriver/README.md:83 (Diff revision 1) > + … > + } > + } > + > + let extension_routes = vec![ > + Method::Get, "/session/{sessionId}/moz/hello", Is there a missing `(` here? The line below has an unbalanced `)`
Attachment #8904045 -
Flags: review?(dburns) → review+
Comment 23•7 years ago
|
||
mozreview-review |
Comment on attachment 8904046 [details] Bug 1368265 - Remove unnecessary license files. https://reviewboard.mozilla.org/r/175780/#review180994
Attachment #8904046 -
Flags: review?(dburns) → review+
Comment 24•7 years ago
|
||
mozreview-review |
Comment on attachment 8904047 [details] Bug 1368265 - Add testing/webdriver hgignore file. https://reviewboard.mozilla.org/r/175782/#review180996
Attachment #8904047 -
Flags: review?(dburns) → review+
Comment 25•7 years ago
|
||
mozreview-review |
Comment on attachment 8904048 [details] Bug 1368265 - Remove outdated contribution advice for geckodriver. https://reviewboard.mozilla.org/r/175784/#review181000 I can't seem to find contribution advice for Geckodriver on Github or MDN only usage advice.
Attachment #8904048 -
Flags: review?(dburns) → review-
Assignee | ||
Comment 26•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8904045 [details] Bug 1368265 - Provide a webdriver README. https://reviewboard.mozilla.org/r/175778/#review180992 > Is there a missing `(` here? The line below has an unbalanced `)` Well spotted.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8904048 -
Attachment is obsolete: true
Assignee | ||
Comment 33•7 years ago
|
||
(In reply to David Burns :automatedtester from comment #25) > Comment on attachment 8904048 [details] Bug 1368265 - Remove outdated > contribution advice for geckodriver. > > https://reviewboard.mozilla.org/r/175784/#review181000 > > I can't seem to find contribution advice for Geckodriver on Github > or MDN only usage advice. Most of the information in testing/geckodriver/CONTRIBUTING.md concerns checking out the repository from GitHub and working with that. None of that is relevant now that the project lives in m-c. The first section about filing good issue reports arguably is, however. So in the interests of landing this I have dropped this commit and will file a follow-up to correct the information in CONTRIBUTING.md.
Comment 34•7 years ago
|
||
Pushed by atolfsen@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/29486614c67c Use in-tree testing/webdriver. r=automatedtester https://hg.mozilla.org/integration/autoland/rev/82ee54d395bb Remove third_party/rust/webdriver. r=automatedtester https://hg.mozilla.org/integration/autoland/rev/ad94237d3b32 Update geckodriver and webdriver Cargo.tomls. r=automatedtester https://hg.mozilla.org/integration/autoland/rev/4d2853618098 Provide a webdriver README. r=automatedtester https://hg.mozilla.org/integration/autoland/rev/04786fb4329f Remove unnecessary license files. r=automatedtester https://hg.mozilla.org/integration/autoland/rev/fb3c4b567fe2 Add testing/webdriver hgignore file. r=automatedtester
Comment 35•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/29486614c67c https://hg.mozilla.org/mozilla-central/rev/82ee54d395bb https://hg.mozilla.org/mozilla-central/rev/ad94237d3b32 https://hg.mozilla.org/mozilla-central/rev/4d2853618098 https://hg.mozilla.org/mozilla-central/rev/04786fb4329f https://hg.mozilla.org/mozilla-central/rev/fb3c4b567fe2
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in
before you can comment on or make changes to this bug.
Description
•