Fix path issues with source maps #117
Conversation
|
@vladikoff: can you add Appveyor properly? This is supposed to fix the Windows issue yet we can't check automatically. @riaann: I still get 5 failures, the same failures we get on master https://ci.appveyor.com/project/gruntjs/grunt-contrib-concat/build/job/k0laci6yydq2q5fb |
|
@XhmikosR: Thanks, I'll have a look at those tests and report back. |
|
The unit tests now all pass on my (Windows) machine. I hope Appveyor agrees! 😃 |
|
@riaann: unfortunately, AppVeyor isn't set up properly yet for this repo so we don't have it for PRs. But the tests seem to pass so gj! PS. Please squash those 2 patches into one. |
On windows, node's path api returns backslashes for path separators. Because the source-map library expects forward slashes in all cases, their relative path logic (specifically their "normalize" function) gives incorrect results when passing in backslash. The mozilla/source-map library won't change this, because they are actually expecting URLs as input - see mozilla/source-map#91 (comment). This fix is similar to the one made for grunt-contrib-uglify: gruntjs/grunt-contrib-uglify#175. Standardized line endings for test fixtures by enforcing LF end of files for fixtures directory (through repository .gitattributes). This change was needed because the expected output of source maps are based on input files with LF, rather than CRLF. Before this change the tests were breaking due to git autocrlf on Windows. The difference between the source maps generated from input files using CRLF and the source maps generated from input files using only LF can't be normalized in the test, due to the nature of the changes: the offsets in the source map are changed, as well as the embedded source. These kinds of changes aren't as simple to normalize as just line feeds in output files - and such normalization would be too invasive, to the point of making the test less effective. This should fix issue gruntjs#110 and gruntjs#95 and all unit tests should now pass.
riaann
force-pushed
the
path-separator-fix
branch
from
b3a3543
to
ac88c56
Compare
|
I have squashed the commits, and consolidated the commit message. |
|
@vladikoff: we really need AppVeyor set up properly so that the PRs are handled too. /CC @sindresorhus for review too. Things seem to work fine from my tests so far. |
|
@XhmikosR fixed! |
|
@vladikoff: thanks! Too bad only owners can trigger a new build :/ |
|
@riaann @sindresorhus: how about using https://github.com/sindresorhus/slash? |
|
Unfortunately, we can't use that library, because it sometimes keeps the Do you agree?
|
|
Yes, @riaann is right. |
|
OK, cool. How about this PR itself then @sindresorhus? |
Fix path issues with source maps
On Windows, node's path api returns backslashes for path separators.
Because the source-map library expects forward slashes in all cases, their relative path logic (specifically their "normalize" function) gives incorrect results when passing in backslash. The mozilla/source-map library won't change this, because they are actually expecting URLs as input - see mozilla/source-map#91 (comment).
This fix is similar to the one made for grunt-contrib-uglify: gruntjs/grunt-contrib-uglify#175.
Fixes #95.