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

fix: path construction errors on Windows #19

Merged
merged 6 commits into from
Nov 18, 2022

Conversation

justinchuby
Copy link
Contributor

@justinchuby justinchuby commented Nov 15, 2022

  • Fix a path construction issue in git.rs and another one in persistent_data.rs.
  • Run CI test on windows and macos.
  • Skip some tests on windows

fixes #18

@justinchuby justinchuby changed the title Run CI on windows and macos Run CI test on windows and macos Nov 15, 2022
@justinchuby
Copy link
Contributor Author

justinchuby commented Nov 15, 2022

I can't seem to figure out the integration tests. It must be the paths somewhere but I am not sure where

@suo
Copy link
Owner

suo commented Nov 16, 2022

the integration tests are snapshotted against linux output—looks like windows gives slightly different error messages

@justinchuby
Copy link
Contributor Author

There are error code 123 file path invalid errors: I suspect path construction is breaking somewhere

@justinchuby
Copy link
Contributor Author

justinchuby commented Nov 16, 2022

Looks like its happening at

lintrunner/src/path.rs

Lines 44 to 47 in 1c991af

fn try_from(p: &String) -> Result<Self> {
Ok(AbsPath {
inner: PathBuf::from(p).canonicalize()?,
})

an input

"p = C:\\Users\\JUSTIN~1\\AppData\\Local\\Temp\\test-lintrunner-configPewVvf.toml"

was turned into

"canonicalize = \\\\?\\C:\\Users\\justinchu\\AppData\\Local\\Temp\\test-lintrunner-configPewVvf.toml"

which then fails

let persistent_data_store = PersistentDataStore::new(&config_path, run_info)?;

hmm looks like this is expected

@justinchuby
Copy link
Contributor Author

"cur_run_dir: \"C:\\\\Users\\\\justinchu\\\\AppData\\\\Roaming\\\\lintrunner\\\\data\\\\e8d5572c7f8a113744a9cc4430ab30c8c7c5382a8459383a6b8e227d4b198a7e\\\\runs\\\\2022-11-16T09:39:09.794-08:00_e229899eed90c4abf0ad1da8bbdfaada91890b91966337def78ab9dad2fea417\""

this is an invalid dir in windows

@justinchuby justinchuby changed the title Run CI test on windows and macos fix: path construction errors on Windows Nov 17, 2022
@justinchuby
Copy link
Contributor Author

@suo I think I fixed it. Please take a look. Thanks!

@suo suo merged commit 032bea6 into suo:main Nov 18, 2022
@suo
Copy link
Owner

suo commented Nov 18, 2022

LGTM, thanks! I'd love to get the windows tests enabled at some point but it shouldn't block for now.

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.

lintrunner panics on Windows
2 participants