Skip to content

Display environment variables for rustc commands - #6492

Merged
bors merged 2 commits into
masterfrom
unknown repository
Jan 3, 2019
Merged

Display environment variables for rustc commands#6492
bors merged 2 commits into
masterfrom
unknown repository

Conversation

@ghost

Copy link
Copy Markdown

This picks up on the work done in PR #5683.

The extra output is only displayed with -vv.

The Windows output has the form set FOO=foo && BAR=bar rustc ... instead of
the form that suggested in #5683 to make escaping easier and since it's
simpler.

This picks up on the work done in PR #5683.
The extra output is only displayed with `-vv`.
The Windows output has the form `set FOO=foo && BAR=bar rustc ...` instead of
the form that suggested in #5683 to make escaping easier and since it's
simpler.
@rust-highfive

Copy link
Copy Markdown

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ehuss (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@Eh2406

Copy link
Copy Markdown
Contributor

Real question, Does that form work? Opening CMD I get:

>set FOO=foo && BAR=bar rustc
'BAR' is not recognized as an internal or external command,
operable program or batch file.

@ehuss

Copy link
Copy Markdown
Contributor

@Eh2406 You need set before each variable.

This doesn't work very well on Windows. The escaping rules of cmd are finicky. For example, set FOO='bar' is embedding the single quotes within the variable (cmd doesn't use single quotes for strings). Also, it is space sensitive, so set FOO=one && BAR=two && rustc... will embed spaces after each variable (FOO is one ). I think avoiding the quotes, and just escaping special characters (like ^") might be an option. And avoid the space between commands (not sure if there's a way to avoid that).

Also, would it be possible to detect the shell and use different syntax? I suspect cmd users are rare (at least I don't know any). Most people I've seen use bash or powershell.

Remove the space at the end of the set commands.
@ghost

Copy link
Copy Markdown
Author

@Eh2406

set FOO=foo && BAR=bar rustc

That was a mistake in my description, I acutally implemented set FOO=foo && set BAR=bar && rustc.

@ehuss

Also, it is space sensitive, so set FOO=one && BAR=two && rustc... will embed spaces after each variable (FOO is one ).

This should be fixed now.

I think avoiding the quotes, and just escaping special characters (like ^") might be an option.

So if I understand you, I should not shell escape the variables and only escape the cmd.exe metacharacters with ^. So "Ben & Jerry's" becomes ^"Ben ^& Jerry^'s^". This seemed to work when I tried it but I'm not sure about this. ProcessBuilder already escapes rustc arguments using shell_escape::escape and doesn't do this (although maybe it should). This article linked from the shell_escape docs seems to indicate that both types of escaping are required.

Also, would it be possible to detect the shell and use different syntax? I suspect cmd users are rare (at least I don't know any). Most people I've seen use bash or powershell.

This is out of scope for me. I have a feeling that the powershell escaping story might be even worse than cmd.exe.

@ehuss

Copy link
Copy Markdown
Contributor

This seemed to work when I tried it but I'm not sure about this.

I am not sure about it, either. However, from my understanding of cmd syntax, it works a bit differently from command-line parsing syntax. It seems to be more reliable to use ^ escaping. I think the argument escaping is probably already correct, since that goes through a different mechanism.

One thing I can't figure out is how to set an empty variable. set FOO= deletes the variable. I don't think cargo has any variables where that would matter, but it doesn't seem quite right.

This is out of scope for me. I have a feeling that the powershell escaping story might be even worse than cmd.exe.

That's fair.

@alexcrichton

Copy link
Copy Markdown
Member

This looks like a great start to me, thanks @mikerite! I agree that we'll probably want powershell syntax eventually, but for now I think it's fine to add this in and iterate from here

@bors: r+

@bors

bors commented Jan 3, 2019

Copy link
Copy Markdown
Contributor

📌 Commit e557f66 has been approved by alexcrichton

@bors

bors commented Jan 3, 2019

Copy link
Copy Markdown
Contributor

⌛ Testing commit e557f66 with merge 34320d2...

bors added a commit that referenced this pull request Jan 3, 2019
Display environment variables for rustc commands
This picks up on the work done in PR #5683.
The extra output is only displayed with `-vv`.
The Windows output has the form `set FOO=foo && BAR=bar rustc ...` instead of
the form that suggested in #5683 to make escaping easier and since it's
simpler.
@bors

bors commented Jan 3, 2019

Copy link
Copy Markdown
Contributor

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 34320d2 to master...

@bors
bors merged commit e557f66 into rust-lang:masterJan 3, 2019
@ehussehuss mentioned this pull request Jan 3, 2019
bors added a commit to rust-lang/rust that referenced this pull request Jan 4, 2019
Update cargo
24 commits in 0d1f1bbeabd5b43a7f3ecfa16540af8e76d5efb4..34320d212dca8cd27d06ce93c16c6151f46fcf2e
2018-12-19 14:45:14 +0000 to 2019-01-03 19:12:38 +0000
- Display environment variables for rustc commands (rust-lang/cargo#6492)
- Fix a very minor race condition in `cargo fix`. (rust-lang/cargo#6515)
- Add a high-level overview of how `fix` works. (rust-lang/cargo#6516)
- Add dependency `registry` to `cargo metadata`. (rust-lang/cargo#6500)
- Fix fingerprint calculation for patched deps. (rust-lang/cargo#6493)
- serialize version directly (rust-lang/cargo#6512)
- use DYLD_FALLBACK_LIBRARY_PATH for dylib_path_envvar on macOS (rust-lang/cargo#6355)
- Fix error message when resolving dependencies (rust-lang/cargo#6510)
- use PathBuf in cargo metadata (rust-lang/cargo#6511)
- Fixed link to testsuite in CONTRIBUTING.md (rust-lang/cargo#6506)
- Update display of contents of Cargo.toml (rust-lang/cargo#6501)
- Update display of contents of Cargo.toml (rust-lang/cargo#6502)
- Fixup cargo install's help message (rust-lang/cargo#6495)
- testsuite: Require failing commands to check output. (rust-lang/cargo#6497)
- Delete unnecessary 'return' (rust-lang/cargo#6496)
- Fix new unused patch warning. (rust-lang/cargo#6494)
- Some minor documentation changes. (rust-lang/cargo#6481)
- Add `links` to `cargo metadata`. (rust-lang/cargo#6480)
- Salvaged semver work (rust-lang/cargo#6476)
- Warn on unused patches. (rust-lang/cargo#6470)
- don't write a an incorrect rustc version to the fingerprint file (rust-lang/cargo#6473)
- Rewrite `login` and registry cleanups. (rust-lang/cargo#6466)
- [issue#6461] Fix cargo commands list (rust-lang/cargo#6462)
- Restrict registry names to same style as package names. (rust-lang/cargo#6469)
@ehussehuss added this to the 1.33.0 milestone Feb 6, 2022
Sign up for freeto 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.

5 participants

@rust-highfive@Eh2406@ehuss@alexcrichton@bors
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Display environment variables for rustc commands · Pull Request #6492 · rust-lang/cargo · GitHub
Skip to content

Display environment variables for rustc commands - #6492

Merged
bors merged 2 commits into
masterfrom
unknown repository
Jan 3, 2019
Merged

Display environment variables for rustc commands#6492
bors merged 2 commits into
masterfrom
unknown repository

Conversation

@ghost

Copy link
Copy Markdown

This picks up on the work done in PR #5683.

The extra output is only displayed with -vv.

The Windows output has the form set FOO=foo && BAR=bar rustc ... instead of
the form that suggested in #5683 to make escaping easier and since it's
simpler.

This picks up on the work done in PR #5683.
The extra output is only displayed with `-vv`.
The Windows output has the form `set FOO=foo && BAR=bar rustc ...` instead of
the form that suggested in #5683 to make escaping easier and since it's
simpler.
@rust-highfive

Copy link
Copy Markdown

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ehuss (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@Eh2406

Copy link
Copy Markdown
Contributor

Real question, Does that form work? Opening CMD I get:

>set FOO=foo && BAR=bar rustc
'BAR' is not recognized as an internal or external command,
operable program or batch file.

@ehuss

Copy link
Copy Markdown
Contributor

@Eh2406 You need set before each variable.

This doesn't work very well on Windows. The escaping rules of cmd are finicky. For example, set FOO='bar' is embedding the single quotes within the variable (cmd doesn't use single quotes for strings). Also, it is space sensitive, so set FOO=one && BAR=two && rustc... will embed spaces after each variable (FOO is one ). I think avoiding the quotes, and just escaping special characters (like ^") might be an option. And avoid the space between commands (not sure if there's a way to avoid that).

Also, would it be possible to detect the shell and use different syntax? I suspect cmd users are rare (at least I don't know any). Most people I've seen use bash or powershell.

Remove the space at the end of the set commands.
@ghost

Copy link
Copy Markdown
Author

@Eh2406

set FOO=foo && BAR=bar rustc

That was a mistake in my description, I acutally implemented set FOO=foo && set BAR=bar && rustc.

@ehuss

Also, it is space sensitive, so set FOO=one && BAR=two && rustc... will embed spaces after each variable (FOO is one ).

This should be fixed now.

I think avoiding the quotes, and just escaping special characters (like ^") might be an option.

So if I understand you, I should not shell escape the variables and only escape the cmd.exe metacharacters with ^. So "Ben & Jerry's" becomes ^"Ben ^& Jerry^'s^". This seemed to work when I tried it but I'm not sure about this. ProcessBuilder already escapes rustc arguments using shell_escape::escape and doesn't do this (although maybe it should). This article linked from the shell_escape docs seems to indicate that both types of escaping are required.

Also, would it be possible to detect the shell and use different syntax? I suspect cmd users are rare (at least I don't know any). Most people I've seen use bash or powershell.

This is out of scope for me. I have a feeling that the powershell escaping story might be even worse than cmd.exe.

@ehuss

Copy link
Copy Markdown
Contributor

This seemed to work when I tried it but I'm not sure about this.

I am not sure about it, either. However, from my understanding of cmd syntax, it works a bit differently from command-line parsing syntax. It seems to be more reliable to use ^ escaping. I think the argument escaping is probably already correct, since that goes through a different mechanism.

One thing I can't figure out is how to set an empty variable. set FOO= deletes the variable. I don't think cargo has any variables where that would matter, but it doesn't seem quite right.

This is out of scope for me. I have a feeling that the powershell escaping story might be even worse than cmd.exe.

That's fair.

@alexcrichton

Copy link
Copy Markdown
Member

This looks like a great start to me, thanks @mikerite! I agree that we'll probably want powershell syntax eventually, but for now I think it's fine to add this in and iterate from here

@bors: r+

@bors

bors commented Jan 3, 2019

Copy link
Copy Markdown
Contributor

📌 Commit e557f66 has been approved by alexcrichton

@bors

bors commented Jan 3, 2019

Copy link
Copy Markdown
Contributor

⌛ Testing commit e557f66 with merge 34320d2...

bors added a commit that referenced this pull request Jan 3, 2019
Display environment variables for rustc commands
This picks up on the work done in PR #5683.
The extra output is only displayed with `-vv`.
The Windows output has the form `set FOO=foo && BAR=bar rustc ...` instead of
the form that suggested in #5683 to make escaping easier and since it's
simpler.
@bors

bors commented Jan 3, 2019

Copy link
Copy Markdown
Contributor

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 34320d2 to master...

@bors
bors merged commit e557f66 into rust-lang:masterJan 3, 2019
@ehussehuss mentioned this pull request Jan 3, 2019
bors added a commit to rust-lang/rust that referenced this pull request Jan 4, 2019
Update cargo
24 commits in 0d1f1bbeabd5b43a7f3ecfa16540af8e76d5efb4..34320d212dca8cd27d06ce93c16c6151f46fcf2e
2018-12-19 14:45:14 +0000 to 2019-01-03 19:12:38 +0000
- Display environment variables for rustc commands (rust-lang/cargo#6492)
- Fix a very minor race condition in `cargo fix`. (rust-lang/cargo#6515)
- Add a high-level overview of how `fix` works. (rust-lang/cargo#6516)
- Add dependency `registry` to `cargo metadata`. (rust-lang/cargo#6500)
- Fix fingerprint calculation for patched deps. (rust-lang/cargo#6493)
- serialize version directly (rust-lang/cargo#6512)
- use DYLD_FALLBACK_LIBRARY_PATH for dylib_path_envvar on macOS (rust-lang/cargo#6355)
- Fix error message when resolving dependencies (rust-lang/cargo#6510)
- use PathBuf in cargo metadata (rust-lang/cargo#6511)
- Fixed link to testsuite in CONTRIBUTING.md (rust-lang/cargo#6506)
- Update display of contents of Cargo.toml (rust-lang/cargo#6501)
- Update display of contents of Cargo.toml (rust-lang/cargo#6502)
- Fixup cargo install's help message (rust-lang/cargo#6495)
- testsuite: Require failing commands to check output. (rust-lang/cargo#6497)
- Delete unnecessary 'return' (rust-lang/cargo#6496)
- Fix new unused patch warning. (rust-lang/cargo#6494)
- Some minor documentation changes. (rust-lang/cargo#6481)
- Add `links` to `cargo metadata`. (rust-lang/cargo#6480)
- Salvaged semver work (rust-lang/cargo#6476)
- Warn on unused patches. (rust-lang/cargo#6470)
- don't write a an incorrect rustc version to the fingerprint file (rust-lang/cargo#6473)
- Rewrite `login` and registry cleanups. (rust-lang/cargo#6466)
- [issue#6461] Fix cargo commands list (rust-lang/cargo#6462)
- Restrict registry names to same style as package names. (rust-lang/cargo#6469)
@ehussehuss added this to the 1.33.0 milestone Feb 6, 2022
Sign up for freeto 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.

5 participants

@rust-highfive@Eh2406@ehuss@alexcrichton@bors
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Display environment variables for rustc commands · Pull Request #6492 · rust-lang/cargo · GitHub
Skip to content

Display environment variables for rustc commands - #6492

Merged
bors merged 2 commits into
masterfrom
unknown repository
Jan 3, 2019
Merged

Display environment variables for rustc commands#6492
bors merged 2 commits into
masterfrom
unknown repository

Conversation

@ghost

Copy link
Copy Markdown

This picks up on the work done in PR #5683.

The extra output is only displayed with -vv.

The Windows output has the form set FOO=foo && BAR=bar rustc ... instead of
the form that suggested in #5683 to make escaping easier and since it's
simpler.

This picks up on the work done in PR #5683.
The extra output is only displayed with `-vv`.
The Windows output has the form `set FOO=foo && BAR=bar rustc ...` instead of
the form that suggested in #5683 to make escaping easier and since it's
simpler.
@rust-highfive

Copy link
Copy Markdown

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ehuss (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@Eh2406

Copy link
Copy Markdown
Contributor

Real question, Does that form work? Opening CMD I get:

>set FOO=foo && BAR=bar rustc
'BAR' is not recognized as an internal or external command,
operable program or batch file.

@ehuss

Copy link
Copy Markdown
Contributor

@Eh2406 You need set before each variable.

This doesn't work very well on Windows. The escaping rules of cmd are finicky. For example, set FOO='bar' is embedding the single quotes within the variable (cmd doesn't use single quotes for strings). Also, it is space sensitive, so set FOO=one && BAR=two && rustc... will embed spaces after each variable (FOO is one ). I think avoiding the quotes, and just escaping special characters (like ^") might be an option. And avoid the space between commands (not sure if there's a way to avoid that).

Also, would it be possible to detect the shell and use different syntax? I suspect cmd users are rare (at least I don't know any). Most people I've seen use bash or powershell.

Remove the space at the end of the set commands.
@ghost

Copy link
Copy Markdown
Author

@Eh2406

set FOO=foo && BAR=bar rustc

That was a mistake in my description, I acutally implemented set FOO=foo && set BAR=bar && rustc.

@ehuss

Also, it is space sensitive, so set FOO=one && BAR=two && rustc... will embed spaces after each variable (FOO is one ).

This should be fixed now.

I think avoiding the quotes, and just escaping special characters (like ^") might be an option.

So if I understand you, I should not shell escape the variables and only escape the cmd.exe metacharacters with ^. So "Ben & Jerry's" becomes ^"Ben ^& Jerry^'s^". This seemed to work when I tried it but I'm not sure about this. ProcessBuilder already escapes rustc arguments using shell_escape::escape and doesn't do this (although maybe it should). This article linked from the shell_escape docs seems to indicate that both types of escaping are required.

Also, would it be possible to detect the shell and use different syntax? I suspect cmd users are rare (at least I don't know any). Most people I've seen use bash or powershell.

This is out of scope for me. I have a feeling that the powershell escaping story might be even worse than cmd.exe.

@ehuss

Copy link
Copy Markdown
Contributor

This seemed to work when I tried it but I'm not sure about this.

I am not sure about it, either. However, from my understanding of cmd syntax, it works a bit differently from command-line parsing syntax. It seems to be more reliable to use ^ escaping. I think the argument escaping is probably already correct, since that goes through a different mechanism.

One thing I can't figure out is how to set an empty variable. set FOO= deletes the variable. I don't think cargo has any variables where that would matter, but it doesn't seem quite right.

This is out of scope for me. I have a feeling that the powershell escaping story might be even worse than cmd.exe.

That's fair.

@alexcrichton

Copy link
Copy Markdown
Member

This looks like a great start to me, thanks @mikerite! I agree that we'll probably want powershell syntax eventually, but for now I think it's fine to add this in and iterate from here

@bors: r+

@bors

bors commented Jan 3, 2019

Copy link
Copy Markdown
Contributor

📌 Commit e557f66 has been approved by alexcrichton

@bors

bors commented Jan 3, 2019

Copy link
Copy Markdown
Contributor

⌛ Testing commit e557f66 with merge 34320d2...

bors added a commit that referenced this pull request Jan 3, 2019
Display environment variables for rustc commands
This picks up on the work done in PR #5683.
The extra output is only displayed with `-vv`.
The Windows output has the form `set FOO=foo && BAR=bar rustc ...` instead of
the form that suggested in #5683 to make escaping easier and since it's
simpler.
@bors

bors commented Jan 3, 2019

Copy link
Copy Markdown
Contributor

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 34320d2 to master...

@bors
bors merged commit e557f66 into rust-lang:masterJan 3, 2019
@ehussehuss mentioned this pull request Jan 3, 2019
bors added a commit to rust-lang/rust that referenced this pull request Jan 4, 2019
Update cargo
24 commits in 0d1f1bbeabd5b43a7f3ecfa16540af8e76d5efb4..34320d212dca8cd27d06ce93c16c6151f46fcf2e
2018-12-19 14:45:14 +0000 to 2019-01-03 19:12:38 +0000
- Display environment variables for rustc commands (rust-lang/cargo#6492)
- Fix a very minor race condition in `cargo fix`. (rust-lang/cargo#6515)
- Add a high-level overview of how `fix` works. (rust-lang/cargo#6516)
- Add dependency `registry` to `cargo metadata`. (rust-lang/cargo#6500)
- Fix fingerprint calculation for patched deps. (rust-lang/cargo#6493)
- serialize version directly (rust-lang/cargo#6512)
- use DYLD_FALLBACK_LIBRARY_PATH for dylib_path_envvar on macOS (rust-lang/cargo#6355)
- Fix error message when resolving dependencies (rust-lang/cargo#6510)
- use PathBuf in cargo metadata (rust-lang/cargo#6511)
- Fixed link to testsuite in CONTRIBUTING.md (rust-lang/cargo#6506)
- Update display of contents of Cargo.toml (rust-lang/cargo#6501)
- Update display of contents of Cargo.toml (rust-lang/cargo#6502)
- Fixup cargo install's help message (rust-lang/cargo#6495)
- testsuite: Require failing commands to check output. (rust-lang/cargo#6497)
- Delete unnecessary 'return' (rust-lang/cargo#6496)
- Fix new unused patch warning. (rust-lang/cargo#6494)
- Some minor documentation changes. (rust-lang/cargo#6481)
- Add `links` to `cargo metadata`. (rust-lang/cargo#6480)
- Salvaged semver work (rust-lang/cargo#6476)
- Warn on unused patches. (rust-lang/cargo#6470)
- don't write a an incorrect rustc version to the fingerprint file (rust-lang/cargo#6473)
- Rewrite `login` and registry cleanups. (rust-lang/cargo#6466)
- [issue#6461] Fix cargo commands list (rust-lang/cargo#6462)
- Restrict registry names to same style as package names. (rust-lang/cargo#6469)
@ehussehuss added this to the 1.33.0 milestone Feb 6, 2022
Sign up for freeto 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.

5 participants

@rust-highfive@Eh2406@ehuss@alexcrichton@bors
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Display environment variables for rustc commands · Pull Request #6492 · rust-lang/cargo · GitHub
Skip to content

Display environment variables for rustc commands - #6492

Merged
bors merged 2 commits into
masterfrom
unknown repository
Jan 3, 2019
Merged

Display environment variables for rustc commands#6492
bors merged 2 commits into
masterfrom
unknown repository

Conversation

@ghost

Copy link
Copy Markdown

This picks up on the work done in PR #5683.

The extra output is only displayed with -vv.

The Windows output has the form set FOO=foo && BAR=bar rustc ... instead of
the form that suggested in #5683 to make escaping easier and since it's
simpler.

This picks up on the work done in PR #5683.
The extra output is only displayed with `-vv`.
The Windows output has the form `set FOO=foo && BAR=bar rustc ...` instead of
the form that suggested in #5683 to make escaping easier and since it's
simpler.
@rust-highfive

Copy link
Copy Markdown

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ehuss (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@Eh2406

Copy link
Copy Markdown
Contributor

Real question, Does that form work? Opening CMD I get:

>set FOO=foo && BAR=bar rustc
'BAR' is not recognized as an internal or external command,
operable program or batch file.

@ehuss

Copy link
Copy Markdown
Contributor

@Eh2406 You need set before each variable.

This doesn't work very well on Windows. The escaping rules of cmd are finicky. For example, set FOO='bar' is embedding the single quotes within the variable (cmd doesn't use single quotes for strings). Also, it is space sensitive, so set FOO=one && BAR=two && rustc... will embed spaces after each variable (FOO is one ). I think avoiding the quotes, and just escaping special characters (like ^") might be an option. And avoid the space between commands (not sure if there's a way to avoid that).

Also, would it be possible to detect the shell and use different syntax? I suspect cmd users are rare (at least I don't know any). Most people I've seen use bash or powershell.

Remove the space at the end of the set commands.
@ghost

Copy link
Copy Markdown
Author

@Eh2406

set FOO=foo && BAR=bar rustc

That was a mistake in my description, I acutally implemented set FOO=foo && set BAR=bar && rustc.

@ehuss

Also, it is space sensitive, so set FOO=one && BAR=two && rustc... will embed spaces after each variable (FOO is one ).

This should be fixed now.

I think avoiding the quotes, and just escaping special characters (like ^") might be an option.

So if I understand you, I should not shell escape the variables and only escape the cmd.exe metacharacters with ^. So "Ben & Jerry's" becomes ^"Ben ^& Jerry^'s^". This seemed to work when I tried it but I'm not sure about this. ProcessBuilder already escapes rustc arguments using shell_escape::escape and doesn't do this (although maybe it should). This article linked from the shell_escape docs seems to indicate that both types of escaping are required.

Also, would it be possible to detect the shell and use different syntax? I suspect cmd users are rare (at least I don't know any). Most people I've seen use bash or powershell.

This is out of scope for me. I have a feeling that the powershell escaping story might be even worse than cmd.exe.

@ehuss

Copy link
Copy Markdown
Contributor

This seemed to work when I tried it but I'm not sure about this.

I am not sure about it, either. However, from my understanding of cmd syntax, it works a bit differently from command-line parsing syntax. It seems to be more reliable to use ^ escaping. I think the argument escaping is probably already correct, since that goes through a different mechanism.

One thing I can't figure out is how to set an empty variable. set FOO= deletes the variable. I don't think cargo has any variables where that would matter, but it doesn't seem quite right.

This is out of scope for me. I have a feeling that the powershell escaping story might be even worse than cmd.exe.

That's fair.

@alexcrichton

Copy link
Copy Markdown
Member

This looks like a great start to me, thanks @mikerite! I agree that we'll probably want powershell syntax eventually, but for now I think it's fine to add this in and iterate from here

@bors: r+

@bors

bors commented Jan 3, 2019

Copy link
Copy Markdown
Contributor

📌 Commit e557f66 has been approved by alexcrichton

@bors

bors commented Jan 3, 2019

Copy link
Copy Markdown
Contributor

⌛ Testing commit e557f66 with merge 34320d2...

bors added a commit that referenced this pull request Jan 3, 2019
Display environment variables for rustc commands
This picks up on the work done in PR #5683.
The extra output is only displayed with `-vv`.
The Windows output has the form `set FOO=foo && BAR=bar rustc ...` instead of
the form that suggested in #5683 to make escaping easier and since it's
simpler.
@bors

bors commented Jan 3, 2019

Copy link
Copy Markdown
Contributor

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 34320d2 to master...

@bors
bors merged commit e557f66 into rust-lang:masterJan 3, 2019
@ehussehuss mentioned this pull request Jan 3, 2019
bors added a commit to rust-lang/rust that referenced this pull request Jan 4, 2019
Update cargo
24 commits in 0d1f1bbeabd5b43a7f3ecfa16540af8e76d5efb4..34320d212dca8cd27d06ce93c16c6151f46fcf2e
2018-12-19 14:45:14 +0000 to 2019-01-03 19:12:38 +0000
- Display environment variables for rustc commands (rust-lang/cargo#6492)
- Fix a very minor race condition in `cargo fix`. (rust-lang/cargo#6515)
- Add a high-level overview of how `fix` works. (rust-lang/cargo#6516)
- Add dependency `registry` to `cargo metadata`. (rust-lang/cargo#6500)
- Fix fingerprint calculation for patched deps. (rust-lang/cargo#6493)
- serialize version directly (rust-lang/cargo#6512)
- use DYLD_FALLBACK_LIBRARY_PATH for dylib_path_envvar on macOS (rust-lang/cargo#6355)
- Fix error message when resolving dependencies (rust-lang/cargo#6510)
- use PathBuf in cargo metadata (rust-lang/cargo#6511)
- Fixed link to testsuite in CONTRIBUTING.md (rust-lang/cargo#6506)
- Update display of contents of Cargo.toml (rust-lang/cargo#6501)
- Update display of contents of Cargo.toml (rust-lang/cargo#6502)
- Fixup cargo install's help message (rust-lang/cargo#6495)
- testsuite: Require failing commands to check output. (rust-lang/cargo#6497)
- Delete unnecessary 'return' (rust-lang/cargo#6496)
- Fix new unused patch warning. (rust-lang/cargo#6494)
- Some minor documentation changes. (rust-lang/cargo#6481)
- Add `links` to `cargo metadata`. (rust-lang/cargo#6480)
- Salvaged semver work (rust-lang/cargo#6476)
- Warn on unused patches. (rust-lang/cargo#6470)
- don't write a an incorrect rustc version to the fingerprint file (rust-lang/cargo#6473)
- Rewrite `login` and registry cleanups. (rust-lang/cargo#6466)
- [issue#6461] Fix cargo commands list (rust-lang/cargo#6462)
- Restrict registry names to same style as package names. (rust-lang/cargo#6469)
@ehussehuss added this to the 1.33.0 milestone Feb 6, 2022
Sign up for freeto 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.

5 participants

@rust-highfive@Eh2406@ehuss@alexcrichton@bors
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' Display environment variables for rustc commands · Pull Request #6492 · rust-lang/cargo · GitHub
Skip to content

Display environment variables for rustc commands - #6492

Merged
bors merged 2 commits into
masterfrom
unknown repository
Jan 3, 2019
Merged

Display environment variables for rustc commands#6492
bors merged 2 commits into
masterfrom
unknown repository

Conversation

@ghost

Copy link
Copy Markdown

This picks up on the work done in PR #5683.

The extra output is only displayed with -vv.

The Windows output has the form set FOO=foo && BAR=bar rustc ... instead of
the form that suggested in #5683 to make escaping easier and since it's
simpler.

This picks up on the work done in PR #5683.
The extra output is only displayed with `-vv`.
The Windows output has the form `set FOO=foo && BAR=bar rustc ...` instead of
the form that suggested in #5683 to make escaping easier and since it's
simpler.
@rust-highfive

Copy link
Copy Markdown

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ehuss (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@Eh2406

Copy link
Copy Markdown
Contributor

Real question, Does that form work? Opening CMD I get:

>set FOO=foo && BAR=bar rustc
'BAR' is not recognized as an internal or external command,
operable program or batch file.

@ehuss

Copy link
Copy Markdown
Contributor

@Eh2406 You need set before each variable.

This doesn't work very well on Windows. The escaping rules of cmd are finicky. For example, set FOO='bar' is embedding the single quotes within the variable (cmd doesn't use single quotes for strings). Also, it is space sensitive, so set FOO=one && BAR=two && rustc... will embed spaces after each variable (FOO is one ). I think avoiding the quotes, and just escaping special characters (like ^") might be an option. And avoid the space between commands (not sure if there's a way to avoid that).

Also, would it be possible to detect the shell and use different syntax? I suspect cmd users are rare (at least I don't know any). Most people I've seen use bash or powershell.

Remove the space at the end of the set commands.
@ghost

Copy link
Copy Markdown
Author

@Eh2406

set FOO=foo && BAR=bar rustc

That was a mistake in my description, I acutally implemented set FOO=foo && set BAR=bar && rustc.

@ehuss

Also, it is space sensitive, so set FOO=one && BAR=two && rustc... will embed spaces after each variable (FOO is one ).

This should be fixed now.

I think avoiding the quotes, and just escaping special characters (like ^") might be an option.

So if I understand you, I should not shell escape the variables and only escape the cmd.exe metacharacters with ^. So "Ben & Jerry's" becomes ^"Ben ^& Jerry^'s^". This seemed to work when I tried it but I'm not sure about this. ProcessBuilder already escapes rustc arguments using shell_escape::escape and doesn't do this (although maybe it should). This article linked from the shell_escape docs seems to indicate that both types of escaping are required.

Also, would it be possible to detect the shell and use different syntax? I suspect cmd users are rare (at least I don't know any). Most people I've seen use bash or powershell.

This is out of scope for me. I have a feeling that the powershell escaping story might be even worse than cmd.exe.

@ehuss

Copy link
Copy Markdown
Contributor

This seemed to work when I tried it but I'm not sure about this.

I am not sure about it, either. However, from my understanding of cmd syntax, it works a bit differently from command-line parsing syntax. It seems to be more reliable to use ^ escaping. I think the argument escaping is probably already correct, since that goes through a different mechanism.

One thing I can't figure out is how to set an empty variable. set FOO= deletes the variable. I don't think cargo has any variables where that would matter, but it doesn't seem quite right.

This is out of scope for me. I have a feeling that the powershell escaping story might be even worse than cmd.exe.

That's fair.

@alexcrichton

Copy link
Copy Markdown
Member

This looks like a great start to me, thanks @mikerite! I agree that we'll probably want powershell syntax eventually, but for now I think it's fine to add this in and iterate from here

@bors: r+

@bors

bors commented Jan 3, 2019

Copy link
Copy Markdown
Contributor

📌 Commit e557f66 has been approved by alexcrichton

@bors

bors commented Jan 3, 2019

Copy link
Copy Markdown
Contributor

⌛ Testing commit e557f66 with merge 34320d2...

bors added a commit that referenced this pull request Jan 3, 2019
Display environment variables for rustc commands
This picks up on the work done in PR #5683.
The extra output is only displayed with `-vv`.
The Windows output has the form `set FOO=foo && BAR=bar rustc ...` instead of
the form that suggested in #5683 to make escaping easier and since it's
simpler.
@bors

bors commented Jan 3, 2019

Copy link
Copy Markdown
Contributor

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 34320d2 to master...

@bors
bors merged commit e557f66 into rust-lang:masterJan 3, 2019
@ehussehuss mentioned this pull request Jan 3, 2019
bors added a commit to rust-lang/rust that referenced this pull request Jan 4, 2019
Update cargo
24 commits in 0d1f1bbeabd5b43a7f3ecfa16540af8e76d5efb4..34320d212dca8cd27d06ce93c16c6151f46fcf2e
2018-12-19 14:45:14 +0000 to 2019-01-03 19:12:38 +0000
- Display environment variables for rustc commands (rust-lang/cargo#6492)
- Fix a very minor race condition in `cargo fix`. (rust-lang/cargo#6515)
- Add a high-level overview of how `fix` works. (rust-lang/cargo#6516)
- Add dependency `registry` to `cargo metadata`. (rust-lang/cargo#6500)
- Fix fingerprint calculation for patched deps. (rust-lang/cargo#6493)
- serialize version directly (rust-lang/cargo#6512)
- use DYLD_FALLBACK_LIBRARY_PATH for dylib_path_envvar on macOS (rust-lang/cargo#6355)
- Fix error message when resolving dependencies (rust-lang/cargo#6510)
- use PathBuf in cargo metadata (rust-lang/cargo#6511)
- Fixed link to testsuite in CONTRIBUTING.md (rust-lang/cargo#6506)
- Update display of contents of Cargo.toml (rust-lang/cargo#6501)
- Update display of contents of Cargo.toml (rust-lang/cargo#6502)
- Fixup cargo install's help message (rust-lang/cargo#6495)
- testsuite: Require failing commands to check output. (rust-lang/cargo#6497)
- Delete unnecessary 'return' (rust-lang/cargo#6496)
- Fix new unused patch warning. (rust-lang/cargo#6494)
- Some minor documentation changes. (rust-lang/cargo#6481)
- Add `links` to `cargo metadata`. (rust-lang/cargo#6480)
- Salvaged semver work (rust-lang/cargo#6476)
- Warn on unused patches. (rust-lang/cargo#6470)
- don't write a an incorrect rustc version to the fingerprint file (rust-lang/cargo#6473)
- Rewrite `login` and registry cleanups. (rust-lang/cargo#6466)
- [issue#6461] Fix cargo commands list (rust-lang/cargo#6462)
- Restrict registry names to same style as package names. (rust-lang/cargo#6469)
@ehussehuss added this to the 1.33.0 milestone Feb 6, 2022
Sign up for freeto 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.

5 participants

@rust-highfive@Eh2406@ehuss@alexcrichton@bors
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Display environment variables for rustc commands · Pull Request #6492 · rust-lang/cargo · GitHub
Skip to content

Display environment variables for rustc commands - #6492

Merged
bors merged 2 commits into
masterfrom
unknown repository
Jan 3, 2019
Merged

Display environment variables for rustc commands#6492
bors merged 2 commits into
masterfrom
unknown repository

Conversation

@ghost

Copy link
Copy Markdown

This picks up on the work done in PR #5683.

The extra output is only displayed with -vv.

The Windows output has the form set FOO=foo && BAR=bar rustc ... instead of
the form that suggested in #5683 to make escaping easier and since it's
simpler.

This picks up on the work done in PR #5683.
The extra output is only displayed with `-vv`.
The Windows output has the form `set FOO=foo && BAR=bar rustc ...` instead of
the form that suggested in #5683 to make escaping easier and since it's
simpler.
@rust-highfive

Copy link
Copy Markdown

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ehuss (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@Eh2406

Copy link
Copy Markdown
Contributor

Real question, Does that form work? Opening CMD I get:

>set FOO=foo && BAR=bar rustc
'BAR' is not recognized as an internal or external command,
operable program or batch file.

@ehuss

Copy link
Copy Markdown
Contributor

@Eh2406 You need set before each variable.

This doesn't work very well on Windows. The escaping rules of cmd are finicky. For example, set FOO='bar' is embedding the single quotes within the variable (cmd doesn't use single quotes for strings). Also, it is space sensitive, so set FOO=one && BAR=two && rustc... will embed spaces after each variable (FOO is one ). I think avoiding the quotes, and just escaping special characters (like ^") might be an option. And avoid the space between commands (not sure if there's a way to avoid that).

Also, would it be possible to detect the shell and use different syntax? I suspect cmd users are rare (at least I don't know any). Most people I've seen use bash or powershell.

Remove the space at the end of the set commands.
@ghost

Copy link
Copy Markdown
Author

@Eh2406

set FOO=foo && BAR=bar rustc

That was a mistake in my description, I acutally implemented set FOO=foo && set BAR=bar && rustc.

@ehuss

Also, it is space sensitive, so set FOO=one && BAR=two && rustc... will embed spaces after each variable (FOO is one ).

This should be fixed now.

I think avoiding the quotes, and just escaping special characters (like ^") might be an option.

So if I understand you, I should not shell escape the variables and only escape the cmd.exe metacharacters with ^. So "Ben & Jerry's" becomes ^"Ben ^& Jerry^'s^". This seemed to work when I tried it but I'm not sure about this. ProcessBuilder already escapes rustc arguments using shell_escape::escape and doesn't do this (although maybe it should). This article linked from the shell_escape docs seems to indicate that both types of escaping are required.

Also, would it be possible to detect the shell and use different syntax? I suspect cmd users are rare (at least I don't know any). Most people I've seen use bash or powershell.

This is out of scope for me. I have a feeling that the powershell escaping story might be even worse than cmd.exe.

@ehuss

Copy link
Copy Markdown
Contributor

This seemed to work when I tried it but I'm not sure about this.

I am not sure about it, either. However, from my understanding of cmd syntax, it works a bit differently from command-line parsing syntax. It seems to be more reliable to use ^ escaping. I think the argument escaping is probably already correct, since that goes through a different mechanism.

One thing I can't figure out is how to set an empty variable. set FOO= deletes the variable. I don't think cargo has any variables where that would matter, but it doesn't seem quite right.

This is out of scope for me. I have a feeling that the powershell escaping story might be even worse than cmd.exe.

That's fair.

@alexcrichton

Copy link
Copy Markdown
Member

This looks like a great start to me, thanks @mikerite! I agree that we'll probably want powershell syntax eventually, but for now I think it's fine to add this in and iterate from here

@bors: r+

@bors

bors commented Jan 3, 2019

Copy link
Copy Markdown
Contributor

📌 Commit e557f66 has been approved by alexcrichton

@bors

bors commented Jan 3, 2019

Copy link
Copy Markdown
Contributor

⌛ Testing commit e557f66 with merge 34320d2...

bors added a commit that referenced this pull request Jan 3, 2019
Display environment variables for rustc commands
This picks up on the work done in PR #5683.
The extra output is only displayed with `-vv`.
The Windows output has the form `set FOO=foo && BAR=bar rustc ...` instead of
the form that suggested in #5683 to make escaping easier and since it's
simpler.
@bors

bors commented Jan 3, 2019

Copy link
Copy Markdown
Contributor

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 34320d2 to master...

@bors
bors merged commit e557f66 into rust-lang:masterJan 3, 2019
@ehussehuss mentioned this pull request Jan 3, 2019
bors added a commit to rust-lang/rust that referenced this pull request Jan 4, 2019
Update cargo
24 commits in 0d1f1bbeabd5b43a7f3ecfa16540af8e76d5efb4..34320d212dca8cd27d06ce93c16c6151f46fcf2e
2018-12-19 14:45:14 +0000 to 2019-01-03 19:12:38 +0000
- Display environment variables for rustc commands (rust-lang/cargo#6492)
- Fix a very minor race condition in `cargo fix`. (rust-lang/cargo#6515)
- Add a high-level overview of how `fix` works. (rust-lang/cargo#6516)
- Add dependency `registry` to `cargo metadata`. (rust-lang/cargo#6500)
- Fix fingerprint calculation for patched deps. (rust-lang/cargo#6493)
- serialize version directly (rust-lang/cargo#6512)
- use DYLD_FALLBACK_LIBRARY_PATH for dylib_path_envvar on macOS (rust-lang/cargo#6355)
- Fix error message when resolving dependencies (rust-lang/cargo#6510)
- use PathBuf in cargo metadata (rust-lang/cargo#6511)
- Fixed link to testsuite in CONTRIBUTING.md (rust-lang/cargo#6506)
- Update display of contents of Cargo.toml (rust-lang/cargo#6501)
- Update display of contents of Cargo.toml (rust-lang/cargo#6502)
- Fixup cargo install's help message (rust-lang/cargo#6495)
- testsuite: Require failing commands to check output. (rust-lang/cargo#6497)
- Delete unnecessary 'return' (rust-lang/cargo#6496)
- Fix new unused patch warning. (rust-lang/cargo#6494)
- Some minor documentation changes. (rust-lang/cargo#6481)
- Add `links` to `cargo metadata`. (rust-lang/cargo#6480)
- Salvaged semver work (rust-lang/cargo#6476)
- Warn on unused patches. (rust-lang/cargo#6470)
- don't write a an incorrect rustc version to the fingerprint file (rust-lang/cargo#6473)
- Rewrite `login` and registry cleanups. (rust-lang/cargo#6466)
- [issue#6461] Fix cargo commands list (rust-lang/cargo#6462)
- Restrict registry names to same style as package names. (rust-lang/cargo#6469)
@ehussehuss added this to the 1.33.0 milestone Feb 6, 2022
Sign up for freeto 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.

5 participants

@rust-highfive@Eh2406@ehuss@alexcrichton@bors
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Display environment variables for rustc commands · Pull Request #6492 · rust-lang/cargo · GitHub
Skip to content

Display environment variables for rustc commands - #6492

Merged
bors merged 2 commits into
masterfrom
unknown repository
Jan 3, 2019
Merged

Display environment variables for rustc commands#6492
bors merged 2 commits into
masterfrom
unknown repository

Conversation

@ghost

Copy link
Copy Markdown

This picks up on the work done in PR #5683.

The extra output is only displayed with -vv.

The Windows output has the form set FOO=foo && BAR=bar rustc ... instead of
the form that suggested in #5683 to make escaping easier and since it's
simpler.

This picks up on the work done in PR #5683.
The extra output is only displayed with `-vv`.
The Windows output has the form `set FOO=foo && BAR=bar rustc ...` instead of
the form that suggested in #5683 to make escaping easier and since it's
simpler.
@rust-highfive

Copy link
Copy Markdown

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ehuss (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@Eh2406

Copy link
Copy Markdown
Contributor

Real question, Does that form work? Opening CMD I get:

>set FOO=foo && BAR=bar rustc
'BAR' is not recognized as an internal or external command,
operable program or batch file.

@ehuss

Copy link
Copy Markdown
Contributor

@Eh2406 You need set before each variable.

This doesn't work very well on Windows. The escaping rules of cmd are finicky. For example, set FOO='bar' is embedding the single quotes within the variable (cmd doesn't use single quotes for strings). Also, it is space sensitive, so set FOO=one && BAR=two && rustc... will embed spaces after each variable (FOO is one ). I think avoiding the quotes, and just escaping special characters (like ^") might be an option. And avoid the space between commands (not sure if there's a way to avoid that).

Also, would it be possible to detect the shell and use different syntax? I suspect cmd users are rare (at least I don't know any). Most people I've seen use bash or powershell.

Remove the space at the end of the set commands.
@ghost

Copy link
Copy Markdown
Author

@Eh2406

set FOO=foo && BAR=bar rustc

That was a mistake in my description, I acutally implemented set FOO=foo && set BAR=bar && rustc.

@ehuss

Also, it is space sensitive, so set FOO=one && BAR=two && rustc... will embed spaces after each variable (FOO is one ).

This should be fixed now.

I think avoiding the quotes, and just escaping special characters (like ^") might be an option.

So if I understand you, I should not shell escape the variables and only escape the cmd.exe metacharacters with ^. So "Ben & Jerry's" becomes ^"Ben ^& Jerry^'s^". This seemed to work when I tried it but I'm not sure about this. ProcessBuilder already escapes rustc arguments using shell_escape::escape and doesn't do this (although maybe it should). This article linked from the shell_escape docs seems to indicate that both types of escaping are required.

Also, would it be possible to detect the shell and use different syntax? I suspect cmd users are rare (at least I don't know any). Most people I've seen use bash or powershell.

This is out of scope for me. I have a feeling that the powershell escaping story might be even worse than cmd.exe.

@ehuss

Copy link
Copy Markdown
Contributor

This seemed to work when I tried it but I'm not sure about this.

I am not sure about it, either. However, from my understanding of cmd syntax, it works a bit differently from command-line parsing syntax. It seems to be more reliable to use ^ escaping. I think the argument escaping is probably already correct, since that goes through a different mechanism.

One thing I can't figure out is how to set an empty variable. set FOO= deletes the variable. I don't think cargo has any variables where that would matter, but it doesn't seem quite right.

This is out of scope for me. I have a feeling that the powershell escaping story might be even worse than cmd.exe.

That's fair.

@alexcrichton

Copy link
Copy Markdown
Member

This looks like a great start to me, thanks @mikerite! I agree that we'll probably want powershell syntax eventually, but for now I think it's fine to add this in and iterate from here

@bors: r+

@bors

bors commented Jan 3, 2019

Copy link
Copy Markdown
Contributor

📌 Commit e557f66 has been approved by alexcrichton

@bors

bors commented Jan 3, 2019

Copy link
Copy Markdown
Contributor

⌛ Testing commit e557f66 with merge 34320d2...

bors added a commit that referenced this pull request Jan 3, 2019
Display environment variables for rustc commands
This picks up on the work done in PR #5683.
The extra output is only displayed with `-vv`.
The Windows output has the form `set FOO=foo && BAR=bar rustc ...` instead of
the form that suggested in #5683 to make escaping easier and since it's
simpler.
@bors

bors commented Jan 3, 2019

Copy link
Copy Markdown
Contributor

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 34320d2 to master...

@bors
bors merged commit e557f66 into rust-lang:masterJan 3, 2019
@ehussehuss mentioned this pull request Jan 3, 2019
bors added a commit to rust-lang/rust that referenced this pull request Jan 4, 2019
Update cargo
24 commits in 0d1f1bbeabd5b43a7f3ecfa16540af8e76d5efb4..34320d212dca8cd27d06ce93c16c6151f46fcf2e
2018-12-19 14:45:14 +0000 to 2019-01-03 19:12:38 +0000
- Display environment variables for rustc commands (rust-lang/cargo#6492)
- Fix a very minor race condition in `cargo fix`. (rust-lang/cargo#6515)
- Add a high-level overview of how `fix` works. (rust-lang/cargo#6516)
- Add dependency `registry` to `cargo metadata`. (rust-lang/cargo#6500)
- Fix fingerprint calculation for patched deps. (rust-lang/cargo#6493)
- serialize version directly (rust-lang/cargo#6512)
- use DYLD_FALLBACK_LIBRARY_PATH for dylib_path_envvar on macOS (rust-lang/cargo#6355)
- Fix error message when resolving dependencies (rust-lang/cargo#6510)
- use PathBuf in cargo metadata (rust-lang/cargo#6511)
- Fixed link to testsuite in CONTRIBUTING.md (rust-lang/cargo#6506)
- Update display of contents of Cargo.toml (rust-lang/cargo#6501)
- Update display of contents of Cargo.toml (rust-lang/cargo#6502)
- Fixup cargo install's help message (rust-lang/cargo#6495)
- testsuite: Require failing commands to check output. (rust-lang/cargo#6497)
- Delete unnecessary 'return' (rust-lang/cargo#6496)
- Fix new unused patch warning. (rust-lang/cargo#6494)
- Some minor documentation changes. (rust-lang/cargo#6481)
- Add `links` to `cargo metadata`. (rust-lang/cargo#6480)
- Salvaged semver work (rust-lang/cargo#6476)
- Warn on unused patches. (rust-lang/cargo#6470)
- don't write a an incorrect rustc version to the fingerprint file (rust-lang/cargo#6473)
- Rewrite `login` and registry cleanups. (rust-lang/cargo#6466)
- [issue#6461] Fix cargo commands list (rust-lang/cargo#6462)
- Restrict registry names to same style as package names. (rust-lang/cargo#6469)
@ehussehuss added this to the 1.33.0 milestone Feb 6, 2022
Sign up for freeto 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.

5 participants

@rust-highfive@Eh2406@ehuss@alexcrichton@bors
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); Display environment variables for rustc commands · Pull Request #6492 · rust-lang/cargo · GitHub
Skip to content

Display environment variables for rustc commands - #6492

Merged
bors merged 2 commits into
masterfrom
unknown repository
Jan 3, 2019
Merged

Display environment variables for rustc commands#6492
bors merged 2 commits into
masterfrom
unknown repository

Conversation

@ghost

Copy link
Copy Markdown

This picks up on the work done in PR #5683.

The extra output is only displayed with -vv.

The Windows output has the form set FOO=foo && BAR=bar rustc ... instead of
the form that suggested in #5683 to make escaping easier and since it's
simpler.

This picks up on the work done in PR #5683.
The extra output is only displayed with `-vv`.
The Windows output has the form `set FOO=foo && BAR=bar rustc ...` instead of
the form that suggested in #5683 to make escaping easier and since it's
simpler.
@rust-highfive

Copy link
Copy Markdown

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ehuss (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@Eh2406

Copy link
Copy Markdown
Contributor

Real question, Does that form work? Opening CMD I get:

>set FOO=foo && BAR=bar rustc
'BAR' is not recognized as an internal or external command,
operable program or batch file.

@ehuss

Copy link
Copy Markdown
Contributor

@Eh2406 You need set before each variable.

This doesn't work very well on Windows. The escaping rules of cmd are finicky. For example, set FOO='bar' is embedding the single quotes within the variable (cmd doesn't use single quotes for strings). Also, it is space sensitive, so set FOO=one && BAR=two && rustc... will embed spaces after each variable (FOO is one ). I think avoiding the quotes, and just escaping special characters (like ^") might be an option. And avoid the space between commands (not sure if there's a way to avoid that).

Also, would it be possible to detect the shell and use different syntax? I suspect cmd users are rare (at least I don't know any). Most people I've seen use bash or powershell.

Remove the space at the end of the set commands.
@ghost

Copy link
Copy Markdown
Author

@Eh2406

set FOO=foo && BAR=bar rustc

That was a mistake in my description, I acutally implemented set FOO=foo && set BAR=bar && rustc.

@ehuss

Also, it is space sensitive, so set FOO=one && BAR=two && rustc... will embed spaces after each variable (FOO is one ).

This should be fixed now.

I think avoiding the quotes, and just escaping special characters (like ^") might be an option.

So if I understand you, I should not shell escape the variables and only escape the cmd.exe metacharacters with ^. So "Ben & Jerry's" becomes ^"Ben ^& Jerry^'s^". This seemed to work when I tried it but I'm not sure about this. ProcessBuilder already escapes rustc arguments using shell_escape::escape and doesn't do this (although maybe it should). This article linked from the shell_escape docs seems to indicate that both types of escaping are required.

Also, would it be possible to detect the shell and use different syntax? I suspect cmd users are rare (at least I don't know any). Most people I've seen use bash or powershell.

This is out of scope for me. I have a feeling that the powershell escaping story might be even worse than cmd.exe.

@ehuss

Copy link
Copy Markdown
Contributor

This seemed to work when I tried it but I'm not sure about this.

I am not sure about it, either. However, from my understanding of cmd syntax, it works a bit differently from command-line parsing syntax. It seems to be more reliable to use ^ escaping. I think the argument escaping is probably already correct, since that goes through a different mechanism.

One thing I can't figure out is how to set an empty variable. set FOO= deletes the variable. I don't think cargo has any variables where that would matter, but it doesn't seem quite right.

This is out of scope for me. I have a feeling that the powershell escaping story might be even worse than cmd.exe.

That's fair.

@alexcrichton

Copy link
Copy Markdown
Member

This looks like a great start to me, thanks @mikerite! I agree that we'll probably want powershell syntax eventually, but for now I think it's fine to add this in and iterate from here

@bors: r+

@bors

bors commented Jan 3, 2019

Copy link
Copy Markdown
Contributor

📌 Commit e557f66 has been approved by alexcrichton

@bors

bors commented Jan 3, 2019

Copy link
Copy Markdown
Contributor

⌛ Testing commit e557f66 with merge 34320d2...

bors added a commit that referenced this pull request Jan 3, 2019
Display environment variables for rustc commands
This picks up on the work done in PR #5683.
The extra output is only displayed with `-vv`.
The Windows output has the form `set FOO=foo && BAR=bar rustc ...` instead of
the form that suggested in #5683 to make escaping easier and since it's
simpler.
@bors

bors commented Jan 3, 2019

Copy link
Copy Markdown
Contributor

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 34320d2 to master...

@bors
bors merged commit e557f66 into rust-lang:masterJan 3, 2019
@ehussehuss mentioned this pull request Jan 3, 2019
bors added a commit to rust-lang/rust that referenced this pull request Jan 4, 2019
Update cargo
24 commits in 0d1f1bbeabd5b43a7f3ecfa16540af8e76d5efb4..34320d212dca8cd27d06ce93c16c6151f46fcf2e
2018-12-19 14:45:14 +0000 to 2019-01-03 19:12:38 +0000
- Display environment variables for rustc commands (rust-lang/cargo#6492)
- Fix a very minor race condition in `cargo fix`. (rust-lang/cargo#6515)
- Add a high-level overview of how `fix` works. (rust-lang/cargo#6516)
- Add dependency `registry` to `cargo metadata`. (rust-lang/cargo#6500)
- Fix fingerprint calculation for patched deps. (rust-lang/cargo#6493)
- serialize version directly (rust-lang/cargo#6512)
- use DYLD_FALLBACK_LIBRARY_PATH for dylib_path_envvar on macOS (rust-lang/cargo#6355)
- Fix error message when resolving dependencies (rust-lang/cargo#6510)
- use PathBuf in cargo metadata (rust-lang/cargo#6511)
- Fixed link to testsuite in CONTRIBUTING.md (rust-lang/cargo#6506)
- Update display of contents of Cargo.toml (rust-lang/cargo#6501)
- Update display of contents of Cargo.toml (rust-lang/cargo#6502)
- Fixup cargo install's help message (rust-lang/cargo#6495)
- testsuite: Require failing commands to check output. (rust-lang/cargo#6497)
- Delete unnecessary 'return' (rust-lang/cargo#6496)
- Fix new unused patch warning. (rust-lang/cargo#6494)
- Some minor documentation changes. (rust-lang/cargo#6481)
- Add `links` to `cargo metadata`. (rust-lang/cargo#6480)
- Salvaged semver work (rust-lang/cargo#6476)
- Warn on unused patches. (rust-lang/cargo#6470)
- don't write a an incorrect rustc version to the fingerprint file (rust-lang/cargo#6473)
- Rewrite `login` and registry cleanups. (rust-lang/cargo#6466)
- [issue#6461] Fix cargo commands list (rust-lang/cargo#6462)
- Restrict registry names to same style as package names. (rust-lang/cargo#6469)
@ehussehuss added this to the 1.33.0 milestone Feb 6, 2022
Sign up for freeto 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.

5 participants

@rust-highfive@Eh2406@ehuss@alexcrichton@bors