Re: [PATCH v5] move rust gitcore crate to a different subdirectory

[Junio C Hamano <gitster@pobox.com>]

Mike Hommey <mh@glandium.org> writes:

> Having `Cargo.toml` at the top-level of the repository implies that one
> can run `cargo build` directly, but this doesn't produce anything useful
> on its own.
>
> Additionally, when including the git source as a submodule of a Rust
> project, it prevents the git source from being included at all in the
> crate package because cargo skips directories that contain a Cargo.toml,
> assuming that everything in the directory is relevant to the crate.
>
> Move all Rust-specific files into a dedicated `rust/` subdirectory.
>
> Signed-off-by: Mike Hommey <mh@glandium.org>
> ---

It would have been a friendly thing to do to describe what base was
chosen, especially with a few other topics in flight that touch the
build procedure for Rust part of the system recently, here below the
three-dash line.  

It seems that this patch is designed to apply cleanly on top of Git
2.56-rc1, which already has these topics merged, so I do not have to
worry about conflicts with them when queueing this patch, which is
good.

>  .gitignore                     |  4 ++--
>  Makefile                       | 24 ++++++++++++------------
>  ci/run-rust-checks.sh          |  6 +++---
>  meson.build                    |  2 +-
>  Cargo.toml => rust/Cargo.toml  |  0
>  build.rs => rust/build.rs      |  0
>  {src => rust}/cargo-meson.sh   |  0
>  {src => rust}/meson.build      | 16 ++++++++--------
>  {src => rust/src}/csum_file.rs |  0
>  {src => rust/src}/hash.rs      |  0
>  {src => rust/src}/lib.rs       |  0
>  {src => rust/src}/loose.rs     |  0
>  {src => rust/src}/varint.rs    |  0
>  13 files changed, 26 insertions(+), 26 deletions(-)
>  rename Cargo.toml => rust/Cargo.toml (100%)
>  rename build.rs => rust/build.rs (100%)
>  rename {src => rust}/cargo-meson.sh (100%)
>  rename {src => rust}/meson.build (81%)
>  rename {src => rust/src}/csum_file.rs (100%)
>  rename {src => rust/src}/hash.rs (100%)
>  rename {src => rust/src}/lib.rs (100%)
>  rename {src => rust/src}/loose.rs (100%)
>  rename {src => rust/src}/varint.rs (100%)

So things in src/ move to either rust/ directory or rust/src/
directory.

> diff --git a/Makefile b/Makefile
> index c649c93c51..67e74c30cc 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -959,7 +959,7 @@ RUST_LIB_NAME = gitcore.lib
>  else
>  RUST_LIB_NAME = libgitcore.a
>  endif
> -RUST_LIB = target$(if $(CARGO_BUILD_TARGET),/$(CARGO_BUILD_TARGET))/$(RUST_BUILD_CONFIG)/$(RUST_LIB_NAME)
> +RUST_LIB = rust/target$(if $(CARGO_BUILD_TARGET),/$(CARGO_BUILD_TARGET))/$(RUST_BUILD_CONFIG)/$(RUST_LIB_NAME)
>  endif

This part was touched by a few topics in the recent past and I
didn't want to resolve conflicts there.  This patch being on top of
these two topics makes my life easier and is very much appreciated.

> @@ -1571,11 +1571,11 @@ CLAR_TEST_OBJS += $(UNIT_TEST_DIR)/unit-test.o
>  
>  UNIT_TEST_OBJS += $(UNIT_TEST_DIR)/test-lib.o
>  
> -RUST_SOURCES += src/csum_file.rs
> -RUST_SOURCES += src/hash.rs
> -RUST_SOURCES += src/lib.rs
> -RUST_SOURCES += src/loose.rs
> -RUST_SOURCES += src/varint.rs
> +RUST_SOURCES += rust/src/csum_file.rs
> +RUST_SOURCES += rust/src/hash.rs
> +RUST_SOURCES += rust/src/lib.rs
> +RUST_SOURCES += rust/src/loose.rs
> +RUST_SOURCES += rust/src/varint.rs

So the sources are all in rust/src/ directory now.

> -$(RUST_LIB): Cargo.toml $(RUST_SOURCES) $(LIB_FILE)
> -	$(QUIET_CARGO)cargo build $(CARGO_ARGS)
> +$(RUST_LIB): rust/Cargo.toml $(RUST_SOURCES) $(LIB_FILE)
> +	$(QUIET_CARGO)cargo build --manifest-path rust/Cargo.toml $(CARGO_ARGS)
> ...
> -RUST_MEMBER_LIBS = $(foreach target,$(RUST_TARGETS),target/$(target)/$(RUST_BUILD_CONFIG)/$(RUST_LIB_NAME))
> -$(RUST_MEMBER_LIBS): target/%/$(RUST_BUILD_CONFIG)/$(RUST_LIB_NAME): Cargo.toml $(RUST_SOURCES) $(LIB_FILE)
> -	$(QUIET_CARGO)cargo build $(CARGO_ARGS) --target $*
> +RUST_MEMBER_LIBS = $(foreach target,$(RUST_TARGETS),rust/target/$(target)/$(RUST_BUILD_CONFIG)/$(RUST_LIB_NAME))
> +$(RUST_MEMBER_LIBS): rust/target/%/$(RUST_BUILD_CONFIG)/$(RUST_LIB_NAME): rust/Cargo.toml $(RUST_SOURCES) $(LIB_FILE)
> +	$(QUIET_CARGO)cargo build --manifest-path rust/Cargo.toml $(CARGO_ARGS) --target $*

Is the reason why we now need to sprinkle --manifest-path all over
is because rust/Cargo.toml is a non-standard place for cargo tool?
Not complaining, but am wondering if it is simpler to set and export
CARGO_MANIFEST_DIR from the Makefile.

> diff --git a/meson.build b/meson.build
> index 0a95d90d21..432e306b21 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1795,7 +1795,7 @@ libgit_sources += version_def_h
>  
>  rust_option = get_option('rust')
>  if rust_option.allowed()
> -  subdir('src')
> +  subdir('rust')

Not 'rust/src'?  Just double-checking.

> @@ -13,7 +13,7 @@ libgit_rs_sources = [
>  cargo_command = [
>    shell,
>    meson.current_source_dir() / 'cargo-meson.sh',
> -  meson.project_source_root(),
> +  meson.current_source_dir(),
>    meson.current_build_dir(),
>  ]

What is this change about?