This is an archive of the discontinued Mercurial Phabricator instance.

Differential D7952

rust: add a README
ClosedPublic

Authored by valentin.gatienbaron on Jan 20 2020, 6:36 PM.

Details

Reviewers
Alphare
pulkit
Group Reviewers
hg-reviewers
Commits
rHGe1b8b4e4f496: rust: add a README
Summary

In particular to explain how to build any of the rust. It's neither
obvious, nor easy to find out, nor easy to determine if you did it
right without some documentation.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Alphare accepted this revision.Jan 21 2020, 4:32 AM
Alphare added a subscriber: Alphare.
Alphare added inline comments.
rust/README.rst
27

I think you're missing the -l option for run-tests.py

34–37

I would add a mention of using cargo c(heck) for faster iteration and using cargo test --all to run the Rust tests, but this patch is already good enough on its own.

pulkit added subscribers: gracinet, pulkit.Jan 21 2020, 7:14 AM
pulkit added inline comments.
rust/README.rst
25–29

IIRC, it's now possible to use HGMODULEPOLICY=rust. @gracinet that's possible now right?

Alphare added inline comments.Jan 21 2020, 7:28 AM
rust/README.rst
25–29

HGMODULEPOLICY=rust+c to select it at runtime yes. But for not for make local AFAIK.

gracinet added a comment.Jan 21 2020, 7:36 AM

My general feeling is that I'd like to deprecate HGWITHRUSTEXT.

We now have python setup.py --rust [build|install|etc] and the coincidental make local PURE=--rust (works because $PURE, while meant to be for --pure is inserted blindly at the right place in the setup.py command.

Maybe this is a good time to decide of a better make variable ? EXTENSION_TYPE ? COMPILE_OPTIONS ? BUILD_OPTIONS ?

It's obviously a good thing to have official instructions, thank you Valentin for raising the subject.

rust/README.rst
25–29

HGMODULEPOLICY is for runtime, it can be rust+c or rust+c-allow.
In the first form, not having the Rust extension is a failure, in the second, there's a fallback.

gracinet added a comment.Jan 21 2020, 7:44 AM

"deprecate" was a bit too strong. Actually I'd like to downgrade HGWITHRUSTEXT to be something that'll be useful to play with alternative ways of building the Rust extension(s). Example: if someone wants to experiment with PyO3.

valentin.gatienbaron marked an inline comment as done.Jan 21 2020, 9:35 AM

I didn't mention setup.py --rust because the toplevel readme doesn't mention setup.py, so I don't really know if that's really how mercurial is intended to be built or an implementation detail.

valentin.gatienbaron added inline comments.Jan 21 2020, 9:38 AM
rust/README.rst
25–29

When I tested, setting HGMODULEPOLICY didn't work, perhaps because of what Raphael said.

27

Well, one can use -l to run rust tests, but it's not mandatory?

gracinet added inline comments.Jan 21 2020, 10:10 AM
rust/README.rst
25–29

Both Raphaël and I were saying the same thin, perhaps a bit partially.

Here's the whole picture:

  • options to make or setup.py govern what is built (HGWITHRUSTEXT also).
  • among the built extensions, what is actually used at runtime depends on the module policy, which is read from mercurial/__modulepolicy__.py and can be overridden by HGMODULEPOLICY ,
  • at build time, a default policy value is set in mercurial/__modulepolicy__.py. For the record, this is rust+c-allow for local builds with Rust and rust+c for other Rust builds.

This policy system is actuallly not specific to Rust, it's also in use with the C extensions, the major difference being that C extensions are built by default.

For example, if one does not build the C extensions and use HGMODULEPOLICY=c, that's a hard error. With HGMODULEPOLICY=allow, one gets a fallback to the pure implementation.

Same with Rust: rust+c gives a hard error is Rust isn't built, while rust+c-allow fallbacks to C extensions only

pulkit accepted this revision.Jan 21 2020, 11:02 AM
This revision is now accepted and ready to land.Jan 21 2020, 11:02 AM
valentin.gatienbaron added a commit: rHGe1b8b4e4f496: rust: add a README.Jan 21 2020, 11:25 AM
Closed by commit rHGe1b8b4e4f496: rust: add a README (authored by valentin.gatienbaron). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mrust/README.rst (47 lines)
Msetup.py (6 lines)

Diff 19473

rust/README.rst

=================== ===================
Mercurial Rust Code Mercurial Rust Code
=================== ===================
This directory contains various Rust code for the Mercurial project. This directory contains various Rust code for the Mercurial project.
Rust is not required to use (or build) Mercurial, but using it
improves performance in some areas.
The top-level ``Cargo.toml`` file defines a workspace containing There are currently three independent rust projects:
all primary Mercurial crates. - chg. An implementation of chg, in rust instead of C.
- hgcli. A experiment for starting hg in rust rather than in python,
by linking with the python runtime. Probably meant to be replaced by
PyOxidizer at some point.
- hg-core (and hg-cpython/hg-directffi): implementation of some
functionality of mercurial in rust, e.g. ancestry computations in
revision graphs or pull discovery. The top-level ``Cargo.toml`` file
defines a workspace containing these crates.
Using hg-core
=============
Local use (you need to clean previous build artifacts if you have
built without rust previously)::
$ HGWITHRUSTEXT=cpython make local # to use ./hg
$ HGWITHRUSTEXT=cpython make tests # to run all tests
$ (cd tests; HGWITHRUSTEXT=cpython ./run-tests.py) # only the .t
AlphareUnsubmitted
Not Done

I think you're missing the -l option for run-tests.py

Alphare: I think you're missing the `-l` option for `run-tests.py`
valentin.gatienbaronAuthorUnsubmitted
Done

Well, one can use -l to run rust tests, but it's not mandatory?

valentin.gatienbaron: Well, one can use `-l` to run rust tests, but it's not mandatory?
$ ./hg debuginstall | grep rust # to validate rust is in use
checking module policy (rust+c-allow)
pulkitUnsubmitted
Not Done

IIRC, it's now possible to use HGMODULEPOLICY=rust. @gracinet that's possible now right?

pulkit: IIRC, it's now possible to use `HGMODULEPOLICY=rust`. @gracinet that's possible now right?
AlphareUnsubmitted
Not Done

HGMODULEPOLICY=rust+c to select it at runtime yes. But for not for make local AFAIK.

Alphare: `HGMODULEPOLICY=rust+c` to select it at runtime yes. But for not for `make local` AFAIK.
gracinetUnsubmitted
Not Done

HGMODULEPOLICY is for runtime, it can be rust+c or rust+c-allow.
In the first form, not having the Rust extension is a failure, in the second, there's a fallback.

gracinet: `HGMODULEPOLICY` is for runtime, it can be `rust+c` or `rust+c-allow`. In the first form, not…
valentin.gatienbaronAuthorUnsubmitted
Done

When I tested, setting HGMODULEPOLICY didn't work, perhaps because of what Raphael said.

valentin.gatienbaron: When I tested, setting HGMODULEPOLICY didn't work, perhaps because of what Raphael said.
gracinetUnsubmitted
Not Done

Both Raphaël and I were saying the same thin, perhaps a bit partially.

Here's the whole picture:

  • options to make or setup.py govern what is built (HGWITHRUSTEXT also).
  • among the built extensions, what is actually used at runtime depends on the module policy, which is read from mercurial/__modulepolicy__.py and can be overridden by HGMODULEPOLICY ,
  • at build time, a default policy value is set in mercurial/__modulepolicy__.py. For the record, this is rust+c-allow for local builds with Rust and rust+c for other Rust builds.

This policy system is actuallly not specific to Rust, it's also in use with the C extensions, the major difference being that C extensions are built by default.

For example, if one does not build the C extensions and use HGMODULEPOLICY=c, that's a hard error. With HGMODULEPOLICY=allow, one gets a fallback to the pure implementation.

Same with Rust: rust+c gives a hard error is Rust isn't built, while rust+c-allow fallbacks to C extensions only

gracinet: Both Raphaël and I were saying the same thin, perhaps a bit partially. Here's the whole…
Building Setting ``HGWITHRUSTEXT`` to other values like ``true`` is deprecated
======== and enables only a fraction of the rust code.
To build the Rust components:: Developing hg-core
==================
$ cargo build Simply run::
AlphareUnsubmitted
Done

I would add a mention of using cargo c(heck) for faster iteration and using cargo test --all to run the Rust tests, but this patch is already good enough on its own.

Alphare: I would add a mention of using `cargo c(heck)` for faster iteration and using `cargo test…
If you prefer a non-debug / release configuration::
$ cargo build --release $ cargo build --release
It is possible to build without ``--release``, but it is not
recommended if performance is of any interest: there can be an order
of magnitude of degradation when removing ``--release``.
For faster builds, you may want to skip code generation::
$ cargo check
You can run only the rust-specific tests (as opposed to tests of
mercurial as a whole) with::
$ cargo test --all

setup.py

"command: %r, environment: %r" "command: %r, environment: %r"
% (self.rustsrcdir, cargocmd, env) % (self.rustsrcdir, cargocmd, env)
) )
class RustEnhancedExtension(RustExtension): class RustEnhancedExtension(RustExtension):
"""A C Extension, conditionally enhanced with Rust code. """A C Extension, conditionally enhanced with Rust code.
If the HGRUSTEXT environment variable is set to something else If the HGWITHRUSTEXT environment variable is set to something else
than 'cpython', the Rust sources get compiled and linked within the than 'cpython', the Rust sources get compiled and linked within
C target shared library object. the C target shared library object.
""" """
def __init__(self, mpath, sources, rustlibname, subcrate, **kw): def __init__(self, mpath, sources, rustlibname, subcrate, **kw):
RustExtension.__init__( RustExtension.__init__(
self, mpath, sources, rustlibname, subcrate, **kw self, mpath, sources, rustlibname, subcrate, **kw
) )
if hgrustext != 'direct-ffi': if hgrustext != 'direct-ffi':
return return