refactor: rename XR to XRSystem

[jsjoeio]

This PR renames XR to XRSystem.

---

• ./mach build -d does not report any errors
• ./mach test-tidy does not report any errors
• These changes fix #25732

• There are tests for these changes OR
• These changes do not require tests because it's the changing of a name (XR -> XRSystem)

[highfive]

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @paulrouget (or someone else) soon.

[highfive]

Heads up! This PR modifies the following files:

• @asajeffrey: components/script/dom/webidls/XR.webidl, components/script/dom/xr.rs, components/script/dom/xrsession.rs
• @KiChjang: components/script/dom/webidls/XR.webidl, components/script/dom/xr.rs, components/script/dom/xrsession.rs

[highfive]

Warning

• These commits modify script code, but no tests are modified. Please consider adding a test!

[jsjoeio]

Running into issues during the build process :(

When I run:

pip install virtualenv

I get this:

Requirement already satisfied: virtualenv in /usr/local/lib/python3.7/site-packages (20.0.2)
Requirement already satisfied: six<2,>=1.12.0 in /usr/local/lib/python3.7/site-packages (from virtualenv) (1.14.0)
Requirement already satisfied: distlib<1,>=0.3.0 in /usr/local/lib/python3.7/site-packages (from virtualenv) (0.3.
0)
Requirement already satisfied: filelock<4,>=3.0.0 in /usr/local/lib/python3.7/site-packages (from virtualenv) (3.0
.12)
Requirement already satisfied: importlib-metadata<2,>=0.12; python_version < "3.8" in /usr/local/lib/python3.7/sit
e-packages (from virtualenv) (1.5.0)
Requirement already satisfied: appdirs<2,>=1.4.3 in /usr/local/lib/python3.7/site-packages (from virtualenv) (1.4.
3)
Requirement already satisfied: zipp>=0.5 in /usr/local/lib/python3.7/site-packages (from importlib-metadata<2,>=0.
12; python_version < "3.8"->virtualenv) (2.2.0)

Then I run this ./mach build -d and get this:

Python virtualenv is not installed. Please install it prior to running mach.

I haven't dabbled in Python in a while so forgive me if this is a n00b question, but what am I doing wrong? It feels like a never ending loop 😂 I will dig deeper tomorrow but initial thought it that it has to do with Python 2.7 vs. Python 3 and the location of virtualenv and which one I'm using.

P.S. - if you do help, or if I figure it out, we can update the README with a "Troubleshooting a Mac environment" section ❤️

[Manishearth]

Can you remove the brewfile changes? Those are for CI. We can land this and perhaps open a second PR to improve docs on this.

[jdm]

With respect to the pip issue, try pip2 install virtualenv or some other binary that explicitly references the python2.7 pip binary, since pip appears to be part of the python3 distribution.

[jsjoeio]

Okay, I removed the brewfile changes.

CI is still failing - but is that simply because I didn't write any unit tests? Looking at the logs, it seems like it's printing the diffs but I don't see any error messages or explanation as to why the task is failing.

[paulrouget]

It’s a formatting issue.

Run mach fmt.

[jsjoeio]

Here's the error from the task:

error[E0583]: file not found for module `XRSystemBinding`
   --> /repo/target/debug/build/script-bbd50c7c9ef66328/out/Bindings/mod.rs:382:9
    |
382 | pub mod XRSystemBinding;
    |         ^^^^^^^^^^^^^^^
    |
    = help: name the file either XRSystemBinding.rs or XRSystemBinding/mod.rs

So it doesn't like the name...best way to fix this?
Resolved

[jsjoeio]

Question: is it possible to set up a pre-push hook that runs ./mach fmt? Wouldn't that save time?

[jsjoeio]

Okay...now this error is a bit more cryptic to me:

error[E0050]: method `RequestSession` has 4 parameters but the declaration in trait `dom::bindings::codegen::Bindings::XRSystemBinding::XRSystemBinding::XRSystemMethods::RequestSession` has 3
   --> components/script/dom/xrsystem.rs:165:9
    |
165 | /         &self,
166 | |         mode: XRSessionMode,
167 | |         init: RootedTraceableBox<XRSessionInit>,
168 | |         comp: InRealm,
    | |_____________________^ expected 3 parameters, found 4
    | 
   ::: /repo/target/debug/build/script-bbd50c7c9ef66328/out/Bindings/XRSystemBinding.rs:893:23
    |
893 |       fn RequestSession(&self, mode: XRSessionMode, parameters: RootedTraceableBox<crate::dom::bindings::codegen::Bindings::XRSystemBinding::XRSessionInit>) -> Rc<Promise>;
    |                         ----------------------------------------------------------------------------------------------------------------------------------- trait requires 3 parameters
error: aborting due to previous error

Any suggestions on how to resolve this?

[jdm]

Ah, that's probably https://github.com/servo/servo/blob/master/components/script/dom/bindings/codegen/Bindings.conf#L143.

[bors-servo]

☔ The latest upstream changes (presumably #25770) made this pull request unmergeable. Please resolve the merge conflicts.

[Manishearth]

Please squash your commits

[jsjoeio]

Thanks for approving! I'll squash now 👍🏼

I'm curious though, what's the advantage to having authors squash their commits as opposed to squashing as part of the PR merge process?

[jdm]

This doesn't build yet:

error[E0405]: cannot find trait `XRMethods` in this scope
   --> components/script/dom/xrsystem.rs:119:6
    |
119 | impl XRMethods for XRSystem {
    |      ^^^^^^^^^ not found in this scope

It needs to implement (and import) XRSystemMethods instead.

As for the question about squashing, we use a bot that performs merges for us and it doesn't support squashing yet.

[jdm]

@bors-servo r=Manishearth,jdm

[bors-servo]

📌 Commit 30f4743 has been approved by Manishearth,jdm

[bors-servo]

⌛ Testing commit 30f4743 with merge fa26253...

[bors-servo]

💔 Test failed - status-taskcluster

[jsjoeio]

[jdm]

@bors-servo retry

[bors-servo]

⌛ Testing commit 30f4743 with merge 5246463...

[bors-servo]

💔 Test failed - status-taskcluster

[CYBAI]

@bors-servo retry

• #24622, #25476

[bors-servo]

⌛ Testing commit 30f4743 with merge 92f5b36...

[bors-servo]

☀️ Test successful - status-taskcluster
Approved by: Manishearth,jdm
Pushing 92f5b36 to master...