Support setting setuid/setgid/sticky in updateMetadata

[phated]

Currently the MASK_MODE constant ignores these but we should probably allow them to be set by .dest().

Ref #151 (comment)

[phated]

@erikkemperman @piranna would either of you have time to look into this?

[tmcgee123]

Although I haven't contributed to this project yet, this looks to be in my range of capabilities (I can write tests as well).

I wanted to get more information on the change so that I could understand the entirety of solution. Here's what I found:

• var MASK_MODE = parseInt('0777', 8); I take it that the beginning 0 is being forgotten since it's parsing as an integer
• the function getMetaData calls getModeDiff, which is using the MASK_MODE
• getModeDiff is not returning correctly due to the integer not being parsed correctly

In this, is the solution to modify getMetaData or getModeDiff? From my perspective, it looks like getModeDiff needs to be altered but I wanted to make sure I know what's going on first.

Hope this helps!

[erikkemperman]

The leading zero indicates an octal number, but is redundant here because the radix 8 is explicitly given. It's just for clarity.

In what way is the integer not parsed correctly? I think this is working as intended...

The easy part of this issue is the change itself (make the mask 7777) but I fear it will be tricky to test (Windows).

[tmcgee123]

Apologies, I never use radix. Parsing that string without the radix in parseInt would give you 777, I wasn't sure how that effected what you were trying to do and what exactly this change entailed. Could you give an example of how .dest() would alter this constant? Or is this change simply change the mask to 7777 and then supporting it with unit tests?

If this is too much for me to take on, that's fine. I've been using Gulp 4 for awhile and was looking for a way to contribute where I could. Thanks for the quick response!

[erikkemperman]

Using octal numbers for privilege/permission bits is a tradition from UNIX, so this way of writing it will be familiar to folks. This is because there are three bits (read, write, execute), and so all combinations of those can be written as a 3-bit number, which is a single digit in octal.

There are digits for "owner", "group" and "others", so that's why the mask is currently 777. This issue is about supporting extended bits for setuid, setgid and sticky.

I think changing the mask to 7777 would basically do the trick, but have no idea how this would work on windows. Note also that dest never changes this constant, it is simply a mask used to filter which bits we care about when getting the modeDiff between the current file and what we want to change it to.

[tmcgee123]

Okay, I think I understand now. Thank you for the explanation. I'll keep ya posted what I come up with if anything!!

[erikkemperman]

Sure thing -- I was asked to take a look at this issue but am completely swamped at work, so I am glad you're willing to try!

Maybe this will help: https://en.wikipedia.org/wiki/File_system_permissions#Traditional_Unix_permissions

[tmcgee123]

So, since Windows does not have file modes, can we just return like we do for isOwner?

If this is true, I've developed a test branch to see if I could solve this. Since 0 is falsey, when getModeDiff is used in updateMetaData it'll just move along as it does now with Windows on isOwner and show that the mode doesn't need to be updated.

There are currently two broken tests I'm seeing from changing the MASK_MODE to 7777, here's the link to the repo:

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

      Uncaught Error: Expected 1 to equal 0
      + expected - actual

      -1
      +0

      at assert (node_modules/expect/lib/assert.js:22:9)
      at node_modules/expect/lib/Expectation.js:103:30
      at addErrorInfo (node_modules/expect/lib/Expectation.js:41:5)
      at Expectation.toEqual (node_modules/expect/lib/Expectation.js:102:7)
      at DestroyableTransform.onEnd (test/destModes.js:352:38)
      at endReadableNT (node_modules/through2/node_modules/readable-stream/lib/_stream_readable.js:865:12)
      at afterTick (node_modules/through2/node_modules/readable-stream/node_modules/process-nextick-args/index.js:18:8)

  2) getModeDiff ignores the sticky/setuid/setgid bits:

      Error: Expected 2560 to equal 0
      + expected - actual

      -2560
      +0

      at assert (node_modules/expect/lib/assert.js:22:9)
      at node_modules/expect/lib/Expectation.js:103:30
      at addErrorInfo (node_modules/expect/lib/Expectation.js:41:5)
      at Expectation.toEqual (node_modules/expect/lib/Expectation.js:102:7)
      at Context.<anonymous> (test/fileOperations.js:179:20)

[phated]

I believe this is done in master now. It will be shipped with 3.0.