1795414 - Publishing geckodriver related crates should only include required files

Status: RESOLVED FIXED

(Testing :: geckodriver, defect, P1)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Yesterday I was releasing geckodriver 0.32.0 (bug 1750691) and had to run cargo publish from within testing/geckodriver to upload the geckodriver crate to crates.io.

Now I tried to build the firefox-source documentation and it failed:

Traceback (most recent call last):
  File "/Users/henrik/code/gecko-obj/opt/_virtualenvs/docs/lib/python3.9/site-packages/sphinx/cmd/build.py", line 276, in build_main
    app = Sphinx(args.sourcedir, args.confdir, args.outputdir,
  File "/Users/henrik/code/gecko-obj/opt/_virtualenvs/docs/lib/python3.9/site-packages/sphinx/application.py", line 237, in __init__
    self.setup_extension(extension)
  File "/Users/henrik/code/gecko-obj/opt/_virtualenvs/docs/lib/python3.9/site-packages/sphinx/application.py", line 394, in setup_extension
    self.registry.load_extension(self, extname)
  File "/Users/henrik/code/gecko-obj/opt/_virtualenvs/docs/lib/python3.9/site-packages/sphinx/registry.py", line 442, in load_extension
    metadata = setup(app)
  File "/Users/henrik/code/gecko/python/mozbuild/mozbuild/sphinx.py", line 251, in setup
    manager.generate_docs(app)
  File "/Users/henrik/code/gecko/tools/moztreedocs/__init__.py", line 99, in generate_docs
    self.trees, self.python_package_dirs = read_build_config(app.srcdir)
  File "/Users/henrik/code/gecko/python/mozbuild/mozbuild/util.py", line 1052, in __call__
    self[args] = self.func(*args)
  File "/Users/henrik/code/gecko/tools/moztreedocs/__init__.py", line 70, in read_build_config
    raise Exception(
Exception: testing/geckodriver has already been registered as a destination.

After adding some debug print statements I got the following details:

**** reldir: target/package/geckodriver-0.32.0 name=SPHINX_TREES key=/testing/geckodriver value=doc
******** absdir=/Users/henrik/code/gecko/target/package/geckodriver-0.32.0/doc
******** updated key=testing/geckodriver
**** reldir: taskcluster name=SPHINX_TREES key=/taskcluster value=docs
******** absdir=/Users/henrik/code/gecko/taskcluster/docs
******** updated key=taskcluster
**** reldir: taskcluster name=SPHINX_PYTHON_PACKAGE_DIRS key=None value=gecko_taskgraph
**** reldir: testing/geckodriver name=SPHINX_TREES key=/testing/geckodriver value=doc
******** absdir=/Users/henrik/code/gecko/testing/geckodriver/doc
******** updated key=testing/geckodriver

As you can see it does not only take testing/geckodriver/docs into account but also target/package/geckodriver-0.32.0. Not sure why that is happening but removing the target/package folder fixed it.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Actually the target/package/geckodriver-0.32.0/ folder is a copy of testing/geckodriver also containing a moz.build file. So I assume this is the reason why it gets registered.

Shall we maybe ignore the target directory for mach doc?

Comment 2

[Andrew Halberstadt [:ahal]]

Ah that makes sense!

It's just traversing the file system looking for moz.build files and I don't know of a way to ignore specific dirs. I'd recommend WONTFIX tbh.

Comment 3

[Andrew Halberstadt [:ahal]]

Also to clarify, this is a problem in the build system, not the documentation infrastructure. I'd summarize it as "File system traversal picks up untracked moz.build files". Though keeping the bug here might be helpful if others run into the same issue?

Edit: I'm pretty sure this would be a WONTFIX from the build perspective too.

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+1]]

So when I'm back from PTO I'll update the geckodriver releasing docs to mention that this folder needs to be deleted. As such moving the bug over to the geckodriver component.

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:whimboo, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Per cargo publish documentation:

This command will create a distributable, compressed .crate file with the source code of the package in the current directory and upload it to a registry.

Given that we have a workspace that is at /target the output of cargo publish is saved there as well including the copy of the geckodriver directory including the moz.build file.

I haven't seen this behavior when we released geckodriver 0.31.0 on April 11th this year. As such I assume it's some kind of new feature.

Mike do you know if there is a way to prevent at least the copy of the source dir, so that our build system doesn't pick up that folder as well due to the extra moz.build file? Or should we prevent the build system to actually traverse the /target folder at all?

Comment 7

[Mike Hommey [:glandium]]

It seems to me you should exclude moz.build from the cargo package, with include or exclude in geckodriver's Cargo.toml.

Comment 8

[Mike Hommey [:glandium]]

(and since I see there's a __pycache__ in my testing/geckodriver directory, you may want to exclude that too)

Comment 9

[Mike Hommey [:glandium]]

likewise for mach_commands.py, which doesn't do much in the crate package.

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Oh that is indeed something that we haven't added yet. Thanks for pointing that out. The relevant documentation can be found at:
https://doc.rust-lang.org/cargo/reference/manifest.html#the-exclude-and-include-fields

There are some more possible files (eg. markdown documentation) that could end-up in the package so I wonder if we should better use the include section and specify what actually should be included.

I wonder what from /testing/geckodriver/ we would actually need. Is the following list correct?

include = ["/.cargo", "/build.rs", "/src"]

Strangely I also have a Cargo.toml.orig file in the final package, which is not be present in the source directory. I'm not sure where this is coming from. Also I assume the package folder should have the target folder with the compiled binary?

Similar include sections would have to be added to other packages:

• marionette
• webdriver
• mozdevice
• mozprofile
• mozrunner
• mozversion

James can you please check if the above is fine for you?

Comment 11

[James Graham [:jgraham]]

You probably also want to include the README.md file since it's referenced from Cargo.toml. But generally this seems fine.

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Bug 1795414 - [geckodriver] Exclude build system files from published packages.

Comment 13

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Bug 1795414 - [geckodriver] Update releasing docs to remove the target/package folder after a release.

Depends on D161125

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+2] from comment #10)

Strangely I also have a Cargo.toml.orig file in the final package, which is not be present in the source directory.

Cargo actually regenerates this file for a best as possible compatibility. As such the original file is placed beside that new file in the same folder.

Comment 15

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:whimboo, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/40a1699890c5
[geckodriver] Exclude build system files from published packages. r=webdriver-reviewers,jgraham
https://hg.mozilla.org/integration/autoland/rev/0926249d2745
[geckodriver] Update releasing docs to remove the target/package folder after a release. r=webdriver-reviewers,jgraham

Comment 17

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/40a1699890c5
https://hg.mozilla.org/mozilla-central/rev/0926249d2745