Fix mode comparison #62

Meroje · 2015-03-06T10:03:02Z

Related to #61, I found that the actual issue is currentMode gets its flags stripped by the bitwise comparison, but not the destination mode. This adds bitwise comparison to the dest mode to fix detection of different modes.

yocontra · 2015-03-06T10:16:48Z

got a test?

Meroje · 2015-03-06T10:52:09Z

Do you mean a test to prove current implementation breaks, or that it works in the PR ?

yocontra · 2015-03-06T15:34:40Z

@Meroje - both, a test that fails currently and passes after this PR

Meroje · 2015-03-09T16:46:23Z

I've got a failing test here, passing on this pull request.

yocontra · 2015-03-09T17:29:02Z

lib/dest/writeContents/index.js

-      if (currentMode === file.stat.mode) {
+      var currentMode = (st.mode & parseInt('0777', 8));
+      var expectedMode = (file.stat.mode & parseInt('0777', 8));
+      if (currentMode === expectedMode) {


hmm this part doesnt click with me. if the mode is the same then the file contents wont get updated?

parseInt is just there for readability, parseInt('07777', 8) would give 4095.
I changed it to 0777 to remove special bits, for example :

given a folder with sticky bit and setuid (5722)

if you then make a file with mode 644, it will become 5644

as of master, currentMode === file.stat.mode evaluates to false

vinyl-fs then tries to chmod the file, which fails when you are not the owner

My opinion is to let the filesystem handle the special bits and just deal with the actual permissions

@Meroje That wasn't my concern at all, not sure where that came from. My concern is that this is skipping file writes if the mode is the same, instead it should just not pass a mode into the file write if the mode is the same

The file is written before the chmod check, written() is the callback of the write* https://github.com/wearefractal/vinyl-fs/blob/ba57ce76e978850f79f78bba9b8aa1a73ac5fb2f/lib/dest/writeContents/index.js#L10-22

Ah sorry I misread the diff - saw line 52 as fs.writeFile

Fix mode comparison

yocontra · 2015-03-22T21:57:47Z

This is causing a failing test for me locally

  1) dest stream should see a file with special chmod (setuid/setgid/sticky) as matching:

      Uncaught AssertionError: expected 978 to be 2002
      + expected - actual

      +2002
      -978

      at Assertion.fail (/Users/contra/Projects/vinyl-fs/node_modules/should/lib/assertion.js:196:17)
      at Assertion.prop.(anonymous function) (/Users/contra/Projects/vinyl-fs/node_modules/should/lib/assertion.js:81:17)
      at DestroyableTransform.onEnd [as _flush] (/Users/contra/Projects/vinyl-fs/test/dest.js:746:56)
      at DestroyableTransform.<anonymous> (/Users/contra/Projects/vinyl-fs/node_modules/through2/node_modules/readable-stream/lib/_stream_transform.js:135:12)
      at DestroyableTransform.g (events.js:257:16)
      at emitNone (events.js:67:13)
      at DestroyableTransform.emit (events.js:163:7)
      at finishMaybe (/Users/contra/Projects/vinyl-fs/node_modules/through2/node_modules/readable-stream/lib/_stream_writable.js:371:12)
      at endWritable (/Users/contra/Projects/vinyl-fs/node_modules/through2/node_modules/readable-stream/lib/_stream_writable.js:378:3)
      at DestroyableTransform.Writable.end (/Users/contra/Projects/vinyl-fs/node_modules/through2/node_modules/readable-stream/lib/_stream_writable.js:356:5)
      at DestroyableTransform.onend (stream.js:60:10)
      at emitNone (events.js:72:20)
      at DestroyableTransform.emit (events.js:163:7)
      at /Users/contra/Projects/vinyl-fs/node_modules/through2/node_modules/readable-stream/lib/_stream_readable.js:965:16
      at process._tickCallback (node.js:349:13)

Meroje · 2015-03-23T06:42:43Z

Just saw it, the the chmod changes to 01722 on stream.end.
The whole thing being to not care about the first bit, I would change the expectedMode or remove the line realMode(fs.lstatSync(expectedPath).mode).should.equal(expectedMode); (was copy/pasted)

yocontra · 2015-03-23T07:23:09Z

@Meroje PR?

Meroje · 2015-03-23T08:08:27Z

It seems this is an osx thing, the test still passes on my ubuntu work machine, I will just remove the chmod equal expectation.

It has been shown in gulpjs#62 this bit can be platform specific and is unreliable, testing fs.chmod is not called will be enough.

It has been shown in #62 this bit can be platform specific and is unreliable, testing fs.chmod is not called will be enough.

Fix mode comparison

It has been shown in #62 this bit can be platform specific and is unreliable, testing fs.chmod is not called will be enough.

Fix mode comparison

0c3516b

Test for chmod with additional bits

ba57ce7

Meroje force-pushed the fix/consistent-mode-comparison branch from ac3fbe3 to ba57ce7 Compare March 9, 2015 16:03

yocontra reviewed Mar 9, 2015
View reviewed changes

yocontra pushed a commit that referenced this pull request Mar 10, 2015

Merge pull request #62 from Meroje/fix/consistent-mode-comparison

96867b4

Fix mode comparison

yocontra merged commit 96867b4 into gulpjs:master Mar 10, 2015

Meroje added a commit to Meroje/vinyl-fs that referenced this pull request Mar 23, 2015

Remove realMode equality test on special bit test

627ace7

It has been shown in gulpjs#62 this bit can be platform specific and is unreliable, testing fs.chmod is not called will be enough.

Meroje mentioned this pull request Mar 23, 2015

Remove realMode equality test on special bit test #65

Merged

phated pushed a commit that referenced this pull request Nov 27, 2017

Remove realMode equality test on special bit test

aa550c7

It has been shown in #62 this bit can be platform specific and is unreliable, testing fs.chmod is not called will be enough.

phated pushed a commit that referenced this pull request Nov 27, 2017

Remove realMode equality test on special bit test

cd18216

It has been shown in #62 this bit can be platform specific and is unreliable, testing fs.chmod is not called will be enough.

phated pushed a commit that referenced this pull request Nov 27, 2017

Remove realMode equality test on special bit test

8dca51b

It has been shown in #62 this bit can be platform specific and is unreliable, testing fs.chmod is not called will be enough.

phated pushed a commit that referenced this pull request Nov 27, 2017

Remove realMode equality test on special bit test

5c8c0d9

It has been shown in #62 this bit can be platform specific and is unreliable, testing fs.chmod is not called will be enough.

phated pushed a commit that referenced this pull request Nov 27, 2017

Remove realMode equality test on special bit test

f08488e

It has been shown in #62 this bit can be platform specific and is unreliable, testing fs.chmod is not called will be enough.

phated pushed a commit that referenced this pull request Nov 27, 2017

Remove realMode equality test on special bit test

43ba817

It has been shown in #62 this bit can be platform specific and is unreliable, testing fs.chmod is not called will be enough.

phated pushed a commit that referenced this pull request Nov 27, 2017

Remove realMode equality test on special bit test

a3c4fd2

It has been shown in #62 this bit can be platform specific and is unreliable, testing fs.chmod is not called will be enough.

phated pushed a commit that referenced this pull request Nov 28, 2017

Remove realMode equality test on special bit test

eb4900e

It has been shown in #62 this bit can be platform specific and is unreliable, testing fs.chmod is not called will be enough.

phated pushed a commit that referenced this pull request Nov 28, 2017

Remove realMode equality test on special bit test

5601c34

It has been shown in #62 this bit can be platform specific and is unreliable, testing fs.chmod is not called will be enough.

phated pushed a commit that referenced this pull request Nov 28, 2017

Remove realMode equality test on special bit test

f039b59

It has been shown in #62 this bit can be platform specific and is unreliable, testing fs.chmod is not called will be enough.

phated pushed a commit that referenced this pull request Nov 28, 2017

Remove realMode equality test on special bit test

dae3a8c

It has been shown in #62 this bit can be platform specific and is unreliable, testing fs.chmod is not called will be enough.

phated pushed a commit that referenced this pull request Nov 28, 2017

Remove realMode equality test on special bit test

2ab33d9

It has been shown in #62 this bit can be platform specific and is unreliable, testing fs.chmod is not called will be enough.

phated pushed a commit that referenced this pull request Nov 28, 2017

Remove realMode equality test on special bit test

714c472

It has been shown in #62 this bit can be platform specific and is unreliable, testing fs.chmod is not called will be enough.

phated pushed a commit that referenced this pull request Nov 28, 2017

Remove realMode equality test on special bit test

a5b32de

It has been shown in #62 this bit can be platform specific and is unreliable, testing fs.chmod is not called will be enough.

phated pushed a commit that referenced this pull request Nov 28, 2017

Remove realMode equality test on special bit test

7f8e34f

It has been shown in #62 this bit can be platform specific and is unreliable, testing fs.chmod is not called will be enough.

phated pushed a commit that referenced this pull request Nov 28, 2017

Remove realMode equality test on special bit test

8a094de

It has been shown in #62 this bit can be platform specific and is unreliable, testing fs.chmod is not called will be enough.

phated pushed a commit that referenced this pull request Nov 28, 2017

Remove realMode equality test on special bit test

8162c66

It has been shown in #62 this bit can be platform specific and is unreliable, testing fs.chmod is not called will be enough.

phated pushed a commit that referenced this pull request Nov 28, 2017

Remove realMode equality test on special bit test

c44db9d

It has been shown in #62 this bit can be platform specific and is unreliable, testing fs.chmod is not called will be enough.

phated pushed a commit that referenced this pull request Nov 28, 2017

Update: Remove realMode equality test on special bit test (ref #62)

1d3a156

phated pushed a commit that referenced this pull request Nov 28, 2017

Update: Remove realMode equality test on special bit test (ref #62)

44ca6bb

phated pushed a commit that referenced this pull request Dec 5, 2017

Merge pull request #62 from Meroje/fix/consistent-mode-comparison

e79e76b

Fix mode comparison

phated pushed a commit that referenced this pull request Dec 5, 2017

Remove realMode equality test on special bit test

08856fa

It has been shown in #62 this bit can be platform specific and is unreliable, testing fs.chmod is not called will be enough.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix mode comparison #62

Fix mode comparison #62

Meroje commented Mar 6, 2015

yocontra commented Mar 6, 2015

Meroje commented Mar 6, 2015

yocontra commented Mar 6, 2015

Meroje commented Mar 9, 2015

yocontra Mar 9, 2015

Meroje Mar 9, 2015

yocontra Mar 9, 2015

Meroje Mar 9, 2015

yocontra Mar 10, 2015

yocontra commented Mar 22, 2015

Meroje commented Mar 23, 2015

yocontra commented Mar 23, 2015

Meroje commented Mar 23, 2015

Fix mode comparison #62

Fix mode comparison #62

Conversation

Meroje commented Mar 6, 2015

yocontra commented Mar 6, 2015

Meroje commented Mar 6, 2015

yocontra commented Mar 6, 2015

Meroje commented Mar 9, 2015

yocontra Mar 9, 2015

Choose a reason for hiding this comment

Meroje Mar 9, 2015

Choose a reason for hiding this comment

yocontra Mar 9, 2015

Choose a reason for hiding this comment

Meroje Mar 9, 2015

Choose a reason for hiding this comment

yocontra Mar 10, 2015

Choose a reason for hiding this comment

yocontra commented Mar 22, 2015

Meroje commented Mar 23, 2015

yocontra commented Mar 23, 2015

Meroje commented Mar 23, 2015