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.

Sign up for GitHub

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

Jump to bottom

[Bug]: rustup always overwrites CARGO_HOME #5013

Open
3 tasks done
marknefedov opened this issue Aug 10, 2023 · 6 comments · May be fixed by #5015
Open
3 tasks done

[Bug]: rustup always overwrites CARGO_HOME #5013

marknefedov opened this issue Aug 10, 2023 · 6 comments · May be fixed by #5015
Labels
bug Something isn't working

Comments

@marknefedov
Copy link

marknefedov commented Aug 10, 2023 •
edited

Prerequisites

  • I have written a descriptive issue title.
  • I have searched all issues/PRs to ensure it has not already been reported or fixed.
  • I have verified that I am using the latest version of Scoop and corresponding bucket.

Package Name

rustup

Expected/Current Behaviour

With the introduction of Windows Dev Drive, I want to move cargo folder as per Microsoft recommendation.
Scoop install always overrides CARGO_HOME, so I can't change folder to custom location.

Steps to Reproduce

setx CARGO_GOME E:/packages/cargo
scoop install rustup
echo $env:CARGO_HOME # This results in C:\Users\user\scoop\persist\rustup\.cargo

Possible Solution

Do not overwrite environment variable

Main/bucket/rustup.json

Lines 33 to 35 in 727f121

"env_set": {
"CARGO_HOME": "$persist_dir\\.cargo",
"RUSTUP_HOME": "$persist_dir\\.rustup"

Pass existing environment variables

Main/bucket/rustup.json

Lines 26 to 29 in 727f121

"script": [
"[Environment]::SetEnvironmentVariable('CARGO_HOME', \"$persist_dir\\.cargo\", 'Process')",
"[Environment]::SetEnvironmentVariable('RUSTUP_HOME', \"$persist_dir\\.rustup\", 'Process')",
"Invoke-ExternalCommand -Path \"$dir\\rustup-init.exe\" -Args @('-y', '--no-modify-path') | Out-Null"

Scoop and Buckets Version

Current Scoop version:
v0.3.1 - Released at 2022-11-15

'extras' bucket:
989df8fb9 (HEAD -> master, origin/master, origin/HEAD) steamcmd: Update to version 1691628584

'java' bucket:
2e9a6179 (HEAD -> master, origin/master, origin/HEAD) temurin11-nightly-jre: Update to version 11.0.21-1.0.202308081844

'main' bucket:
727f1212d (HEAD -> master, origin/master, origin/HEAD) red: Update to version 10aug23

'versions' bucket:
6db6c0f9a (HEAD -> master, origin/master, origin/HEAD) vlc-nightly: Update to version 20230810

Scoop Config

scoop config

last_update         scoop_branch scoop_repo
-----------         ------------ ----------
10/08/2023 10:17:13 master       https://github.com/ScoopInstaller/Scoop

PowerShell Version

$PSVersionTable

Name                           Value
----                           -----
PSVersion                      7.3.6
PSEdition                      Core
GitCommitId                    7.3.6
OS                             Microsoft Windows 10.0.22631
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

Additional Softwares

No response
@marknefedov marknefedov added the bug Something isn't working label Aug 10, 2023
@marknefedov marknefedov changed the title [Bug]: [Bug]: rustup always overwrites CARGO_HOME Aug 10, 2023
@marknefedov marknefedov linked a pull request Aug 10, 2023 that will close this issue
Open
1 task
@HUMORCE
Copy link
Member

HUMORCE commented Aug 10, 2023

For special use cases, you should create custom manifest instead of changing the general solution.

This report are similar to #5006

@rashil2000
Copy link
Member

I think including a check for the existence of those variables is an acceptable solution.

@HUMORCE
Copy link
Member

HUMORCE commented Aug 12, 2023

I think including a check for the existence of those variables is an acceptable solution.

I don't think it's acceptable, when the manifest explicitly states that environment variables will be changed/added/seted:

❯ scoop info rustup

Name        : rustup
...
Environment : CARGO_HOME = <root>\.cargo
              RUSTUP_HOME = <root>\.rustup
Path Added  : <root>\.cargo\bin

For users who run scoop install rustup when they already have a rustup installation, we should assume that them wanting a rustup(Scooped).

Compared to package manager under *nix, Scoop does not (re)build program from source, then: #5006 (comment)

We just missing docs for these issue:

Scoop should follow Principle Of Least Surprise and stick to upstream defaults unless there is a good justification for doing so and in which case it must be documented.

#5006

@hagaigold
Copy link

hagaigold commented Oct 11, 2023 •
edited

For special use cases, you should create custom manifest instead of changing the general solution.

Another option: adding to config.json a customized "env_set" section, that will be loaded after each update/install.
I think it is easier to maintain than a custom manifest.

@rashil2000
Copy link
Member

I think including a check for the existence of those variables is an acceptable solution.

The thing is, there's already a precedent for this: see #4512

@niheaven what do you think?

@niheaven
Copy link
Member

niheaven commented Apr 3, 2024

Prefer a customized manifest here, but yes, I know the benefit of Dev Drive, and may give a config option to overwrite the default behavior of envvar creation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add support for *_HOME variables in rustup. marknefedov/Main
5 participants
@HUMORCE @niheaven @marknefedov @hagaigold @rashil2000