This is an archive of the discontinued Mercurial Phabricator instance.

Differential D1395

treedirstate: create empty Rust project
ClosedPublic

Authored by mbthomas on Nov 14 2017, 12:38 PM.
Tags
None
Subscribers
quark, durham

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
Closed by commit rFBHGX401e19090975: treedirstate: create empty Rust project (authored by mbthomas). · Explain WhyNov 28 2017, 5:59 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 3924

rust/treedirstate/.cargo/config

  • This file was added.
# On OS X targets, configure the linker to perform dynamic lookup of undefined
# symbols. This allows the library to be used as a Python extension.
quarkUnsubmitted
Not Done

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

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

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.

mbthomas: Correct. We get whichever libpython was part of the process that loaded us. These symbols are…
quarkUnsubmitted
Not Done

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?

quark: I saw your comment about cython installation place. I wonder if you're using a wrong Python to…
[target.i686-apple-darwin]
rustflags = ["-C", "link-args=-Wl,-undefined,dynamic_lookup"]
durhamUnsubmitted
Not Done

Do we need a linux target here?

durham: Do we need a linux target here?
mbthomasAuthorUnsubmitted
Not Done

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: On OS X you need to pass extra arguments to the linker to let it produce a library that has…
[target.x86_64-apple-darwin]
rustflags = ["-C", "link-args=-Wl,-undefined,dynamic_lookup"]

rust/treedirstate/Cargo.lock

  • This file was added.
[[package]]
name = "rusttreedirstate"
version = "0.1.0"

rust/treedirstate/Cargo.toml

  • This file was added.
[package]
name = "rusttreedirstate"
version = "0.1.0"
authors = ["Facebook Source Control Team <sourcecontrol-dev@fb.com>"]
[profile.release]
lto = true
[lib]
name = "rusttreedirstate"
crate-type = ["cdylib"]

rust/treedirstate/src/lib.rs

  • This file was added.
// Copyright Facebook, Inc. 2017
//! treedirstate - Tree-based Directory State.
//!
//! This is a Rust implementation of the dirstate concept for Mercurial, using a tree structure
//! in an append-only storage back-end.
//!
//! The directory state stores information for all files in a working copy that are of interest
//! to Mercurial. In particular, for each file in the working copy it stores the mode flags,
//! size, and modification time of the file. These can be compared with current values to
//! determine if the file has changed.
//!
//! The directory state also stores files that are in the working copy parent manifest but have
//! been marked as removed.

treedirstate/Makefile

  • This file was added.
NAME := rusttreedirstate
# Cargo's depfiles use full paths, so we must include the full path
# to the rust library.
DIR := $(realpath $(join $(dir $(firstword $(MAKEFILE_LIST))),../rust/treedirstate))
MODE ?= release
ifeq ($(MODE),release)
CARGO_ARGS += --release
endif
ifeq ($(shell uname),Darwin)
LIBSUFFIX := .dylib
else
LIBSUFFIX := .so
endif
RUSTLIB := $(DIR)/target/$(MODE)/lib$(NAME)$(LIBSUFFIX)
RUSTDEP := $(DIR)/target/$(MODE)/lib$(NAME).d
.PHONY: all
all: $(NAME).so
$(NAME).so: $(RUSTLIB)
cp $< $@
$(RUSTLIB):
cd $(DIR) && cargo build $(CARGO_ARGS)
.PHONY: test
test:
cd $(DIR) && cargo test
.PHONY: clean
clean:
$(RM) $(NAME).so
cd $(DIR) && cargo clean
quarkUnsubmitted
Not Done

Maybe:

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

ignore exit codes.

quark: Maybe: -$(RM) -f $(NAME).so -cd $(DIR) && cargo clean ignore exit codes.
mbthomasAuthorUnsubmitted
Not Done

$(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: `$(RM)` expands to `rm -f`. I think the clean rule should fail if it fails to clean, so that…
# This pattern rule for Rust source files forces a rebuild if any source file
# is deleted.
$(DIR)/src/%.rs: ;
-include $(RUSTDEP)