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

use filepath.Clean() when comparing filenames to include/exclude patterns #1565

Merged
merged 2 commits into from Oct 8, 2016

Conversation

technoweenie
Copy link
Contributor

This removes the windows-specific hacks, and uses filepath.Clean() to sanitize the paths. This broke previously because it was filepath.Match() expects back slashes on windows.

@technoweenie
Copy link
Contributor Author

/cc @ttaylorr @sinbad @sschuberth This is the last thing I need for script/integration to pass on my windows machine.

Copy link
Contributor

@ttaylorr ttaylorr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good. There's some duplication here, but this function has a good amount of it already. 👍

@ttaylorr ttaylorr merged commit 74a10e1 into master Oct 8, 2016
@ttaylorr ttaylorr deleted the windows/fix-fetch-include-exclude branch October 8, 2016 00:12
@sschuberth
Copy link
Member

@technoweenie Thanks for this. Out of curiosity, are you running on Windows 10? Because on Windows 10 I see more test failures than on Windows 8.1. These additional failures seem to be related to Windows short vs. long path issues where Windows 10 seems to behave differently, and I'm trying to address them as part of my windows-tests branch.

@sinbad
Copy link
Contributor

sinbad commented Oct 10, 2016

👍 Looks like the main issue here was people using '' separators in their include/exclude config, which I don't usually advocate (all other git settings tend to work better using '/' even on Windows), but this is good belt & braces. The tests didn't used to fail on Windows for that, maybe something else has been changed in the meantime.

@technoweenie
Copy link
Contributor Author

Out of curiosity, are you running on Windows 10? Because on Windows 10 I see more test failures than on Windows 8.1.

I ran them on my Windows 10 gaming pc :)

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.

None yet

4 participants