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

Relative path resolution on Windows fails #6

Closed
macedigital opened this Issue Jun 26, 2017 · 5 comments

Comments

Projects
None yet
2 participants
@macedigital
Owner

macedigital commented Jun 26, 2017

Relative path resolution on Windows doesn't work, instead tests on Windows produce this trace:

Error: 0 position is not passing assertion: 0 == 3
      at DestroyableTransform.stream.assertion (node_modules\stream-assert\assertStream.js:24:26)
      at DestroyableTransform._transform (node_modules\stream-assert\index.js:102:10)
      at DestroyableTransform.Transform._read (node_modules\stream-assert\node_modules\readable-stream\lib\_stream_transform.js:184:10)
      at DestroyableTransform.Transform._write (node_modules\stream-assert\node_modules\readable-stream\lib\_stream_transform.js:172:12)
      at doWrite (node_modules\stream-assert\node_modules\readable-stream\lib\_stream_writable.js:237:10)
      at writeOrBuffer (node_modules\stream-assert\node_modules\readable-stream\lib\_stream_writable.js:227:5)
      at DestroyableTransform.Writable.write (node_modules\stream-assert\node_modules\readable-stream\lib\_stream_writable.js:194:11)
      at write (node_modules\stream-assert\node_modules\readable-stream\lib\_stream_readable.js:623:24)
      at flow (node_modules\stream-assert\node_modules\readable-stream\lib\_stream_readable.js:632:7)
      at DestroyableTransform.pipeOnReadable (node_modules\stream-assert\node_modules\readable-stream\lib\_stream_readable.js:664:5)
      at emitReadable_ (node_modules\stream-assert\node_modules\readable-stream\lib\_stream_readable.js:448:10)
      at emitReadable (node_modules\stream-assert\node_modules\readable-stream\lib\_stream_readable.js:444:5)
      at readableAddChunk (node_modules\stream-assert\node_modules\readable-stream\lib\_stream_readable.js:187:9)
      at DestroyableTransform.Readable.push (node_modules\stream-assert\node_modules\readable-stream\lib\_stream_readable.js:149:10)
      at DestroyableTransform.Transform.push (node_modules\stream-assert\node_modules\readable-stream\lib\_stream_transform.js:145:32)
      at afterTransform (node_modules\stream-assert\node_modules\readable-stream\lib\_stream_transform.js:101:12)
      at TransformState.afterTransform (node_modules\stream-assert\node_modules\readable-stream\lib\_stream_transform.js:79:12)
      at DestroyableTransform._transform (node_modules\stream-assert\index.js:80:3)
      at DestroyableTransform.Transform._read (node_modules\stream-assert\node_modules\readable-stream\lib\_stream_transform.js:184:10)
      at DestroyableTransform.Transform._write (node_modules\stream-assert\node_modules\readable-stream\lib\_stream_transform.js:172:12)
      at doWrite (node_modules\stream-assert\node_modules\readable-stream\lib\_stream_writable.js:237:10)
      at writeOrBuffer (node_modules\stream-assert\node_modules\readable-stream\lib\_stream_writable.js:227:5)
      at DestroyableTransform.Writable.write (node_modules\stream-assert\node_modules\readable-stream\lib\_stream_writable.js:194:11)
      at DestroyableTransform.ondata (node_modules\readable-stream\lib\_stream_readable.js:611:20)
      at addChunk (node_modules\readable-stream\lib\_stream_readable.js:283:12)
      at readableAddChunk (node_modules\readable-stream\lib\_stream_readable.js:270:11)
      at DestroyableTransform.Readable.push (node_modules\readable-stream\lib\_stream_readable.js:237:10)
      at DestroyableTransform.Transform.push (node_modules\readable-stream\lib\_stream_transform.js:146:32)
      at afterTransform (node_modules\readable-stream\lib\_stream_transform.js:102:51)
      at TransformState.afterTransform (node_modules\readable-stream\lib\_stream_transform.js:79:12)
      at DestroyableTransform.pipeHandler [as _transform] (index.js:128:14)
      at DestroyableTransform.Transform._read (node_modules\readable-stream\lib\_stream_transform.js:182:10)
      at DestroyableTransform.Transform._write (node_modules\readable-stream\lib\_stream_transform.js:170:83)
      at doWrite (node_modules\readable-stream\lib\_stream_writable.js:405:64)
      at writeOrBuffer (node_modules\readable-stream\lib\_stream_writable.js:394:5)
      at DestroyableTransform.Writable.write (node_modules\readable-stream\lib\_stream_writable.js:321:11)
      at write (node_modules\stream-assert\node_modules\readable-stream\lib\_stream_readable.js:623:24)
      at flow (node_modules\stream-assert\node_modules\readable-stream\lib\_stream_readable.js:632:7)
      at DestroyableTransform.pipeOnReadable (node_modules\stream-assert\node_modules\readable-stream\lib\_stream_readable.js:664:5)
      at emitReadable_ (node_modules\stream-assert\node_modules\readable-stream\lib\_stream_readable.js:448:10)
      at emitReadable (node_modules\stream-assert\node_modules\readable-stream\lib\_stream_readable.js:444:5)
      at readableAddChunk (node_modules\stream-assert\node_modules\readable-stream\lib\_stream_readable.js:187:9)
      at DestroyableTransform.Readable.push (node_modules\stream-assert\node_modules\readable-stream\lib\_stream_readable.js:149:10)
      at DestroyableTransform.Transform.push (node_modules\stream-assert\node_modules\readable-stream\lib\_stream_transform.js:145:32)
      at afterTransform (node_modules\stream-assert\node_modules\readable-stream\lib\_stream_transform.js:101:12)
      at TransformState.afterTransform (node_modules\stream-assert\node_modules\readable-stream\lib\_stream_transform.js:79:12)
      at DestroyableTransform._transform (node_modules\stream-assert\index.js:80:3)
      at DestroyableTransform.Transform._read (node_modules\stream-assert\node_modules\readable-stream\lib\_stream_transform.js:184:10)
      at DestroyableTransform.Transform._write (node_modules\stream-assert\node_modules\readable-stream\lib\_stream_transform.js:172:12)
      at doWrite (node_modules\stream-assert\node_modules\readable-stream\lib\_stream_writable.js:237:10)
      at writeOrBuffer (node_modules\stream-assert\node_modules\readable-stream\lib\_stream_writable.js:227:5)
      at DestroyableTransform.Writable.write (node_modules\stream-assert\node_modules\readable-stream\lib\_stream_writable.js:194:11)
      at write (node_modules\vinyl-fs\node_modules\readable-stream\lib\_stream_readable.js:623:24)
      at flow (node_modules\vinyl-fs\node_modules\readable-stream\lib\_stream_readable.js:632:7)
      at DestroyableTransform.pipeOnReadable (node_modules\vinyl-fs\node_modules\readable-stream\lib\_stream_readable.js:664:5)
      at emitReadable_ (node_modules\vinyl-fs\node_modules\readable-stream\lib\_stream_readable.js:448:10)
      at emitReadable (node_modules\vinyl-fs\node_modules\readable-stream\lib\_stream_readable.js:444:5)
      at readableAddChunk (node_modules\vinyl-fs\node_modules\readable-stream\lib\_stream_readable.js:187:9)
      at DestroyableTransform.Readable.push (node_modules\vinyl-fs\node_modules\readable-stream\lib\_stream_readable.js:149:10)
      at DestroyableTransform.Transform.push (node_modules\vinyl-fs\node_modules\readable-stream\lib\_stream_transform.js:145:32)
      at afterTransform (node_modules\vinyl-fs\node_modules\readable-stream\lib\_stream_transform.js:101:12)
      at TransformState.afterTransform (node_modules\vinyl-fs\node_modules\readable-stream\lib\_stream_transform.js:79:12)
      at DestroyableTransform.noop [as _transform] (node_modules\vinyl-fs\node_modules\through2\through2.js:26:3)
      at DestroyableTransform.Transform._read (node_modules\vinyl-fs\node_modules\readable-stream\lib\_stream_transform.js:184:10)
      at DestroyableTransform.Transform._write (node_modules\vinyl-fs\node_modules\readable-stream\lib\_stream_transform.js:172:12)
      at doWrite (node_modules\vinyl-fs\node_modules\readable-stream\lib\_stream_writable.js:237:10)
      at writeOrBuffer (node_modules\vinyl-fs\node_modules\readable-stream\lib\_stream_writable.js:227:5)
      at DestroyableTransform.Writable.write (node_modules\vinyl-fs\node_modules\readable-stream\lib\_stream_writable.js:194:11)
      at write (node_modules\vinyl-fs\node_modules\readable-stream\lib\_stream_readable.js:623:24)
      at flow (node_modules\vinyl-fs\node_modules\readable-stream\lib\_stream_readable.js:632:7)
      at DestroyableTransform.pipeOnReadable (node_modules\vinyl-fs\node_modules\readable-stream\lib\_stream_readable.js:664:5)
      at emitReadable_ (node_modules\vinyl-fs\node_modules\readable-stream\lib\_stream_readable.js:448:10)
      at emitReadable (node_modules\vinyl-fs\node_modules\readable-stream\lib\_stream_readable.js:444:5)
      at readableAddChunk (node_modules\vinyl-fs\node_modules\readable-stream\lib\_stream_readable.js:187:9)
      at DestroyableTransform.Readable.push (node_modules\vinyl-fs\node_modules\readable-stream\lib\_stream_readable.js:149:10)
      at DestroyableTransform.Transform.push (node_modules\vinyl-fs\node_modules\readable-stream\lib\_stream_transform.js:145:32)
      at afterTransform (node_modules\vinyl-fs\node_modules\readable-stream\lib\_stream_transform.js:101:12)
      at TransformState.afterTransform (node_modules\vinyl-fs\node_modules\readable-stream\lib\_stream_transform.js:79:12)
      at node_modules\vinyl-fs\lib\src\getContents\bufferFile.js:12:5
      at FSReqWrap.readFileAfterClose [as oncomplete] (fs.js:446:3)

@macedigital macedigital added the bug label Jun 26, 2017

@macedigital macedigital self-assigned this Jun 26, 2017

@XhmikosR

This comment has been minimized.

Contributor

XhmikosR commented Jun 26, 2017

I tried solving this the other day I was fiddling with the repo to no avail. I might be able to have a look again soon-ish if you don't beat me to it.

@macedigital

This comment has been minimized.

Owner

macedigital commented Jun 26, 2017

Two issues prevented tests to pass on Windows:

  • Tests for relative path resolution were not portable and checked path separators by forward-slash (but Windows uses backward slashes). Commit ead6130 fixes this issue.

  • Hash checksum calculation yields different results if files in question use CRLF line endings. Therefore checksums didn't match and tests failed. Commit 7ec45d8 forces checking out css- and js-files with LF file endings.

@XhmikosR Can you pull the latest changes from master, and run tests again? You may have to manually convert line-endings (or do a fresh new checkout). This time it should work, though 😄

Also, I updated the README.md with a note about this line-ending issue (6620fa4).

@XhmikosR

This comment has been minimized.

Contributor

XhmikosR commented Jun 26, 2017

@macedigital: Looks good to me. A couple of thoughts:

  • I would remove the init section from AppVeyor; you shouldn't rely on such things
  • An alternative approach to .gitattributes would be to make sure you just do foo.replace(/\r\n/g, '\n');
@XhmikosR

This comment has been minimized.

Contributor

XhmikosR commented Jun 26, 2017

Oh, an irrelevant thing, but I don't want to make a new issue.

Make sure you use the files section in package.json so that you don't include unneeded files when publishing. Like:

  "files": [
    "index.js"
  ]
@macedigital

This comment has been minimized.

Owner

macedigital commented Jun 26, 2017

@XhmikosR Thanks for your help! And don't hesitate to open tickets ;)

Regarding your feedback:
I removed the init section from the appveyor.yml, it's definitively not needed.

Line-ending settings in .gitattributes are local to this module, and in a sense, only affect the test fixtures. In practice, it shouldn't actually matter with which line endings files are saved with, as long they are never converted.

Also, I'll consider limiting the number of files for the next release (e.g. excluding test files).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment