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

neither hide nor hide-env seems to hide environment variables from externals #7937

Closed
fdncred opened this issue Feb 1, 2023 · 10 comments · Fixed by #12901
Closed

neither hide nor hide-env seems to hide environment variables from externals #7937

fdncred opened this issue Feb 1, 2023 · 10 comments · Fixed by #12901
Assignees
Labels
🐛 bug Something isn't working scoping/name-resolution How Nu finds which variables/functions are in scope and to what they are bound
Milestone

Comments

@fdncred
Copy link
Collaborator

fdncred commented Feb 1, 2023

Describe the bug

I'm trying to hide an environment variable from externals, specifically cargo.

How to reproduce

  1. hide CARGO_TARGET_DIR
  2. hide-env CARGO_TARGET_DIR
  3. $env.CARGO_TARGET_DIR produces the error it should
  4. ^env show that CARGO_TARGET_DIR is still set
  5. cargo build, puts all build artifacts in the CARGO_TARGET_DIR folder

Expected behavior

Externals shouldn't see a hidden env var

Screenshots

No response

Configuration

key value
version 0.75.1
branch main
commit_hash 5e957ec
build_os windows-x86_64
build_target x86_64-pc-windows-msvc
rust_version rustc 1.66.1 (90743e729 2023-01-10)
rust_channel 1.66.1-x86_64-pc-windows-msvc
cargo_version cargo 1.66.1 (ad779e08b 2023-01-10)
pkg_version 0.75.1
build_time 2023-02-01 08:23:51 -06:00
build_rust_channel release
features database, dataframe, default, trash, which, zip
installed_plugins custom-value generate

Additional context

No response

@fdncred fdncred added the 🐛 bug Something isn't working label Feb 1, 2023
@refl3ction
Copy link

Any updates?

@fdncred
Copy link
Collaborator Author

fdncred commented Feb 10, 2023

I think I've narrowed it down to this line returning None. Now to figure out what to do about it.

self.env_hidden
.insert(active_overlay.into(), HashSet::from([name.into()]));

@kubouch
Copy link
Contributor

kubouch commented Feb 12, 2023

@fdncred What is the output of your overlay list at the time you're calling the hide-env?

kubouch added a commit that referenced this issue Feb 12, 2023
# Description

This one fixes env not being hidden inside closure, reported in the
conversation under #6593

#6593
#7937 still persist. These
seems a bit more involved and might need hidden env tracking also in the
engine state... I'm not yet sure what's causing it.

Also re-enables some env-related tests and removes unused Value clone.

# User-Facing Changes

Just a bugfix

# Tests + Formatting

Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A
clippy::needless_collect` to check that you're using the standard code
style
- `cargo test --workspace` to check that all tests pass

# After Submitting

If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.
@fdncred
Copy link
Collaborator Author

fdncred commented Feb 12, 2023

@fdncred What is the output of your overlay list at the time you're calling the hide-env?

╭───┬──────╮
│ 0 │ zero │
╰───┴──────╯

@fdncred
Copy link
Collaborator Author

fdncred commented Feb 12, 2023

I only use this env var on Windows and on Windows by default shell is pwsh. CARGO_TARGET_DIR is set the regular way you set env vars in Windows. I can't remember right now if it's in HKLM or HKCU. But I'm wondering if it has something to do with having a parent shell and inheriting those env vars?

@fdncred
Copy link
Collaborator Author

fdncred commented Feb 12, 2023

I can reproduce it on my mac too like this.

In fish:

> echo $COLORTERM
truecolor

start nushell

> $env.COLORTERM
truecolor

now do this

> hide-env COLORTERM
> $env.COLORTERM
Error: nu::shell::column_not_found (link)

  × Cannot find column
   ╭─[entry #5:1:1]
 1 │ $env.COLORTERM
   · ──┬─ ────┬────
   ·   │      ╰── cannot find column 'COLORTERM'
   ·   ╰── value originates here
   ╰────
> ^env 
CMD_DURATION_MS=1
COLORTERM=truecolor

@kubouch
Copy link
Contributor

kubouch commented Feb 12, 2023

Yeah, it might be the inherited env vars... I thought it had to do with multiple overlays but couldn't reproduce it and since you have only one, the problem will be somewhere else.

@kubouch
Copy link
Contributor

kubouch commented Feb 12, 2023

What is truecolor?

@fdncred
Copy link
Collaborator Author

fdncred commented Feb 12, 2023

it's an env var that some *nix terminals need to support 24-bit color vs 8-bit.

@fdncred
Copy link
Collaborator Author

fdncred commented Sep 26, 2023

similar to #6593 but different examples.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working scoping/name-resolution How Nu finds which variables/functions are in scope and to what they are bound
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants