-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Find the program using PATHEXT #37381
Closed
afiune
wants to merge
10
commits into
rust-lang:master
from
afiune:afiune/37380/find_program_using_pathext
Closed
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
40ed572
Find the program using PATHEXT
cc9d064
Added env_lookup fn
a3c72fd
Added tests for Command::env_lookup()
63b4c79
Hoist the PATHEXT env lookup
f364190
Fixed some nitpicks for better code quality
41a0df0
Added process_command tests for find_program
6b9b6f1
On batch files set ApplicationName to `cmd.exe`
abe5e1a
Remove find_program tests
0444a6d
Add as_vec_u16 fn
e9e960e
Add tests for make_command_line modifications
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,80 @@ | ||
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT | ||
// file at the top-level directory of this distribution and at | ||
// http://rust-lang.org/COPYRIGHT. | ||
// | ||
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or | ||
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license | ||
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your | ||
// option. This file may not be copied, modified, or distributed | ||
// except according to those terms. | ||
|
||
#[cfg(test)] | ||
mod test_find_program { | ||
use std::ffi::{OsStr, OsString}; | ||
use std::process::Command; | ||
use std::collections::HashMap; | ||
use std::path::Path; | ||
use std::fs::canonicalize; | ||
use std::env::join_paths; | ||
|
||
fn gen_env() -> HashMap<OsString, OsString> { | ||
let env: HashMap<OsString, OsString> = HashMap::new(); | ||
env.insert(OsString::from("HOMEDRIVE"), OsString::from("C:")); | ||
let p1 = canonicalize("./src/test/run-pass/process_command/fixtures/bin").unwrap(); | ||
let p2 = canonicalize("./src/test/run-pass/process_command/fixtures").unwrap(); | ||
let p3 = canonicalize("./src/test/run-pass/process_command").unwrap(); | ||
let paths = vec![p1, p2, p3]; | ||
let path = join_paths(paths).unwrap(); | ||
env.insert(OsString::from("PATH"), OsString::from(&path)); | ||
env.insert(OsString::from("USERNAME"), OsString::from("rust")); | ||
env | ||
} | ||
|
||
fn command_with_pathext(cmd: &str) -> Command { | ||
let mut env = gen_env(); | ||
env.insert( | ||
OsString::from("PATHEXT"), | ||
OsString::from(".COM;.EXE;.BAT;.CMD;.VBS;.VBE;.MSC") | ||
); | ||
let mut cmd = Command::new(cmd); | ||
cmd.with_env(&env); | ||
cmd | ||
} | ||
|
||
fn command_without_pathext(cmd: &str) -> Command { | ||
let env = gen_env(); | ||
let mut cmd = Command::new(cmd); | ||
cmd.with_env(&env); | ||
cmd | ||
} | ||
|
||
mod with_pathext_set { | ||
use super::command_with_pathext; | ||
|
||
fn command_on_path_found() { | ||
let c = command_with_pathext("bin"); | ||
let bat = canonicalize("./src/test/run-pass/process_command/fixtures/bin/bin.bat"); | ||
assert_eq!(bat.ok(), c.find_program()); | ||
} | ||
|
||
fn command_not_found() { | ||
let c = command_with_pathext("missing"); | ||
assert_eq!(None, c.find_program()); | ||
} | ||
} | ||
|
||
mod without_pathext_set { | ||
use super::command_without_pathext; | ||
|
||
fn bat_command_not_found() { | ||
let c = command_without_pathext("bin"); | ||
assert_eq!(None, c.find_program()); | ||
} | ||
|
||
fn exe_command_found() { | ||
let c = command_without_pathext("exec"); | ||
let exe = canonicalize("./src/test/run-pass/process_command/fixtures/bin/exec.exe"); | ||
assert_eq!(exe.ok(), c.find_program()); | ||
} | ||
} | ||
} |
Empty file.
Empty file.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm unfortunately the
find_program
function here is a private implementation detail ofCommand
so this function I don't think will pass on Windows :(There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alexcrichton I made it to be public, just as
Command.cmd()
andCommand.env()
.. I wonder if it should be private as you mentioned and instead test within the Module? 🤔There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah the public function you added is actually an internal implementation detail that's wrapped with a
std::process::Command
, so it won't be accessible here.But yeah moving the test to that module would be ok.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome! Im working on it