Page MenuHomePhabricator

treedirstate: create empty Rust project
ClosedPublic

Authored by mbthomas on Nov 14 2017, 12:38 PM.

Details

Reviewers
quark
durham
Group Reviewers
Restricted Project
Commits
rFBHGX401e19090975: treedirstate: create empty Rust project
Summary

Create an empty Rust project for treedirstate. This will be a
re-implementation of the dirstate map using a tree structure, where nodes in
the tree are directories, and leaves are files.

Diff Detail

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

Event Timeline

mbthomas created this revision.Nov 14 2017, 12:38 PM
Herald added a reviewer: Restricted Project. · View Herald TranscriptNov 14 2017, 12:38 PM
durham added a subscriber: durham.Nov 14 2017, 8:11 PM

lgtm, but I don't know rust, so someone who does may want to take a look.

rust/treedirstate/.cargo/config
6

Do we need a linux target here?

mbthomas added inline comments.Nov 15 2017, 12:33 PM
rust/treedirstate/.cargo/config
6

On OS X you need to pass extra arguments to the linker to let it produce a library that has unresolved symbols that need to be resolved dynamically on load. On Linux (and Windows, I believe) that's the default behaviour.

I should probably add a comment, though.

mbthomas updated this revision to Diff 3565.Nov 16 2017, 11:42 AM
quark accepted this revision.Nov 21 2017, 3:28 AM
quark added a subscriber: quark.
quark added inline comments.
rust/treedirstate/.cargo/config
3

Does this mean that at the build time, we don't know which libpython to link against?

treedirstate/Makefile
38–39

Maybe:

-$(RM) -f $(NAME).so
-cd $(DIR) && cargo clean

ignore exit codes.

This revision is now accepted and ready to land.Nov 21 2017, 3:28 AM
mbthomas added inline comments.Nov 21 2017, 5:22 AM
rust/treedirstate/.cargo/config
3

Correct. We get whichever libpython was part of the process that loaded us.

These symbols are all referred to by the rust-cpython crate, and the rust-cpython continuous integration has tests that ensure that only symbols that are definitely available in libpython available are used, so this is ok.

treedirstate/Makefile
38–39

$(RM) expands to rm -f. I think the clean rule should fail if it fails to clean, so that make clean && make all will fail appropriately.

mbthomas updated this revision to Diff 3817.Nov 24 2017, 11:52 AM
quark added inline comments.Nov 24 2017, 1:33 PM
rust/treedirstate/.cargo/config
3

I saw your comment about cython installation place. I wonder if you're using a wrong Python to run setup.py.

Try source hg-dev and it will make python same as what hg.real uses, which is different from the default python used on OSX. If that is used to run setup.py, is the link args still necessary?

durham accepted this revision.Nov 27 2017, 12:23 PM
This revision was automatically updated to reflect the committed changes.