Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rustc: Install compiler with toolchain at spksrc.tc.mk time #5508

Merged
merged 17 commits into from
Dec 20, 2022

Conversation

th0ma7
Copy link
Contributor

@th0ma7 th0ma7 commented Nov 30, 2022

Description

rustc: Install compiler with toolchain at spksrc.tc.mk time

Related to #5527 and #5475

Checklist

  • Build rule all-supported completed successfully
  • New installation of package completed successfully
  • Package upgrade completed successfully (Manually install the package again)
  • Package functionality was tested
  • Any needed documentation is updated/created

Type of change

  • Bug fix
  • New Package
  • Package update
  • Includes small framework changes
  • This change requires a documentation update (e.g. Wiki)

@th0ma7 th0ma7 requested a review from hgy59 November 30, 2022 00:50
@th0ma7
Copy link
Contributor Author

th0ma7 commented Nov 30, 2022

@hgy59 your pair of eye would be needed for this rust toolchain change. You can test the docker side of things from the documentation I added at the end of https://github.com/SynoCommunity/spksrc/wiki/Developers-HOW-TO

EDIT: FYI, this is not fully tested yet but looks promising.

@th0ma7
Copy link
Contributor Author

th0ma7 commented Dec 7, 2022

@hgy59 friendly reminder if cycles allow for it to test this to significantly reduce our docker image size. Testing on my end looked good.

th0ma7 added a commit that referenced this pull request Dec 19, 2022
rustc is now automatically downloaded based on arch being build starting with PR #5508
@th0ma7 th0ma7 self-assigned this Dec 20, 2022
@th0ma7 th0ma7 merged commit fdd169a into SynoCommunity:master Dec 20, 2022
@th0ma7 th0ma7 deleted the rustc_non-priv-user branch December 20, 2022 20:55
@th0ma7 th0ma7 restored the rustc_non-priv-user branch December 20, 2022 21:57
@th0ma7
Copy link
Contributor Author

th0ma7 commented Dec 20, 2022

@hgy59 while I had tested this throughout it seems something isn't right with the rustup initial setup... I'll investigate tonight and if I can't find where the issue is I'll revert the change from master.

@hgy59
Copy link
Contributor

hgy59 commented Dec 20, 2022

@th0ma7 sorry for not providing feedback to this PR, so many WIP...

@th0ma7
Copy link
Contributor Author

th0ma7 commented Dec 21, 2022

@hgy59 I may have found the issue... in docker using priviledge mode it requires:

# Setup the rustc environment
ENV CARGO_HOME=/spksrc/distrib/cargo
ENV RUSTUP_HOME=/spksrc/distrib/rustup
ENV PATH="$VENV_PATH/bin:$RUSTUP_HOME/bin:$CARGO_HOME/bin:$PATH"

It seems I had forgotten to force rebuilding my local docker environment... build currently in progress to confirm rust environment works as expected...

EDIT: I believe it's now fixed on master.

@th0ma7 th0ma7 deleted the rustc_non-priv-user branch December 21, 2022 01:36
@th0ma7
Copy link
Contributor Author

th0ma7 commented Dec 21, 2022

@hgy59 so while this now works in priviledge Docker mode with the added fix, there is now something else in non-priviledge. There probably isn't much missing... Main issue is that everytime you install rustup it pokes your default .profile and .bashrc which in turns makes you believe things are great and all settled... but a fresh re-install shows it isn't. In the meantime, docker on github seems all good at least and I'm probably inches away from fully fixing this.

Expect me to rework that last bit... Although feel free to chim-in and figure out it doesn't work in non-priv mode.

EDIT: Believe I've found it... dam realpath when used where the directory doesn't exist yet thus returning empty string...

$ git diff
diff --git a/mk/spksrc.common.mk b/mk/spksrc.common.mk
index da5243f6..8f65752b 100644
--- a/mk/spksrc.common.mk
+++ b/mk/spksrc.common.mk
@@ -15,9 +15,9 @@ RUN = cd $(WORK_DIR)/$(PKG_DIR) && env $(ENV)
 
 # Add cargo for rust compiler to default PATH
 ifneq ($(BASE_DISTRIB_DIR),)
-export PATH := $(realpath $(BASE_DISTRIB_DIR)/cargo/bin):$(PATH)
-export CARGO_HOME = $(realpath $(BASE_DISTRIB_DIR)/cargo)
-export RUSTUP_HOME = $(realpath $(BASE_DISTRIB_DIR)/rustup)
+export PATH := $(BASE_DISTRIB_DIR)/cargo/bin:$(PATH)
+export CARGO_HOME = $(BASE_DISTRIB_DIR)/cargo
+export RUSTUP_HOME = $(BASE_DISTRIB_DIR)/rustup
 endif
 
 # fallback by default to native/python*

I'll revisit tomorrow and perform full round of testing to confirm all use-cases are still OK and wetter the Dockerfile changes are really needed (which I doubt as I had tested that but probably added realpath really late in the game and didn't saw it's impact as the directories already existed...)

@th0ma7
Copy link
Contributor Author

th0ma7 commented Dec 21, 2022

@hgy59 I believe it's now solved on master, hopefully permanently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants