GNUmakefile: Support CARGO_BUILD_TARGET#9223
Conversation
|
GNU testsuite comparison: |
|
@XhstormR Would you review this? Tests are failing with |
|
GNU testsuite comparison: |
|
@oech3 MacOS' env dont support should use |
This comment was marked as resolved.
This comment was marked as resolved.
6134f2a to
c83f76d
Compare
|
GNU testsuite comparison: |
|
@Ecordonnier You will prefer this approarch instread of adding srange variable. |
|
@oech3 Doesn't I also think it's unnecessary to clear the CARGO_BUILD_TARGET variable. Instead, we should check the CARGO_BUILD_TARGET variable, and if it's set, add it to BUILDDIR, then we don't need to change the uudoc path, as I mentioned before: |
|
For simplity, does |
a9571ed to
2dda1fd
Compare
|
@oech3 add else if check ifdef CARGO_TARGET_DIR
BUILDDIR := $(CARGO_TARGET_DIR)/${PROFILE}
+ else ifdef CARGO_BUILD_TARGET
+ BUILDDIR := $(BASEDIR)/target/$(CARGO_BUILD_TARGET)/${PROFILE}
else
BUILDDIR := $(BASEDIR)/target/${PROFILE}
endif``` |
|
GNU testsuite comparison: |
|
Undefined var is replaced by empty string.
target//${PROFILE} (double slash) should be OK.
|
We need it since we use native |
|
GNU testsuite comparison: |
ef38ab4 to
88f1dd9
Compare
|
GNU testsuite comparison: |
384c335 to
a0c64c2
Compare
Merging this PR will not alter performance
Comparing Footnotes
|
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
|
@sylvestre Is this OK to merge? 0 change for 1 week. |
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
|
ok? |
|
I'm not hyper interested in this PR. But ping. |
| # CARGO_BUILD_TARGET should be undefined for native (non-cross) build. | ||
| BASEDIR ?= $(shell pwd) | ||
| ifdef CARGO_TARGET_DIR | ||
| BUILDDIR := $(CARGO_TARGET_DIR)/${PROFILE} |
There was a problem hiding this comment.
you should define BUILDDIR_UUDOC at lines 49 and 51 and take CARGO_TARGET_DIR into account. The problem with current state of the PR is that:
- "$(BASEDIR)/target/$(PROFILE)/uudoc" is duplicated 4 times
- it doesn't work with CARGO_TARGET_DIR:
env CARGO_TARGET_DIR=foobar CARGO_BUILD_TARGET=aarch64-unknown-linux-gnu make install-manpages PREFIX=/tmp/usr UTILS=true
mkdir -p /tmp/usr/share/man/man1
/home/asteba/dev/coreutils/target/debug/uudoc manpage true > /tmp/usr/share/man/man1/true.1
/bin/sh: 1: /home/asteba/dev/coreutils/target/debug/uudoc: not found
make: *** [GNUmakefile:389: install-manpages] Error 127
...
There was a problem hiding this comment.
ifdef CARGO_TARGET_DIR
BUILDDIR := $(CARGO_TARGET_DIR)/${PROFILE}
BUILDDIR_UUDOC := $(CARGO_TARGET_DIR)/${PROFILE}
else
BUILDDIR := $(BASEDIR)/target/$(CARGO_BUILD_TARGET)/${PROFILE}
BUILDDIR_UUDOC := $(BASEDIR)/target/${PROFILE}
endif
There was a problem hiding this comment.
I have @unset CARGO_BUILD_TARGET && ${CARGO} build ${CARGOFLAGS} --bin uudoc .... Not enough?
I add BUILDDIR_UUDOC in any case.
There was a problem hiding this comment.
It is not enough, setting @unset CARGO_BUILD_TARGET does not have an impact on CARGO_TARGET_DIR. The issue is that the build directory is not under BASEDIR when setting CARGO_TARGET_DIR to another directory.
The PR is a good change. Please fix the one issue I mentioned and I'll merge it. |
|
GNU testsuite comparison: |
|
@oech3 btw can you please correct your git email configuration? Your commits have no email configured for the Author, but they have an email for the Committer. This makes github show two different people for the commits ( see 97f8a08 for instance ): |
|
Can I set both empty? |
I would set both to 79379754+oech3@users.noreply.github.com , I think having an empty email is quite uncommon. |
|
Is @.noreply.* not auto-generated one at each commit? |
No, if you go to https://github.com/settings/emails , at the bottom of the page you see your "public profile email". This email is permanent. |
OK. |
Fixes uutils#9206 . Removes `RUSTC_ARCH` variable.
Fixes uutils#9206 . Removes `RUSTC_ARCH` variable.
Fixes #9206 . Removes
RUSTC_ARCHvariable.