[Default behaviour] Copy permission bits from template to compile target

[JakobGM]

Alacritty should set the permission bits of the compiled template to be equal to the template source file. This hits me as the most reasonable default behaviour in the absence of a specific permissions directive attached to the compile action.

It should be implemented in astrality.compiler.compile_template, specifically in an else statement attached to this if-else control structure.

The testing strategy should follow along the lines of this test.

[sshashank124]

I took a shot at adding the functionality and its corresponding test. I didn't make the relevant changes to the docs files though. Let me know if there's something I should add/remove/change

[JakobGM]

That seems to do the job!

If you want me to be nitpicky, I usually use trailing commas on function invocations which are split over several lines, like this one. I like the consistency of it, and as a added benefit it reduces git diff noice when adding new function parameters in the future. But it is mainly a stylistic choice I have enforced on the rest of the code base.

And, as an addendum, I wonder if you know more about the leading bits of Path.stat().st_mode? I noticed that it consisted of 6 bytes, rather than 3, as I thought it would be. It seems to indicate the type of the file.

With other words, what is the consequense of this line? Let's say that st_mode = 0o010755, would the result be st_mode & 0o777 == 0o000755? I guess chmod only changes the 3 minor bytes? Why should chmod change the type of file, that would be confusing anyway.

Perhaps we should test the behaviour by checking that the leading 3 bytes are unaltered by the function?

[sshashank124]

Yeah the last 3 bytes are for the file permissions and the first 3 bytes are various other flags. I think instead of manually doing & 0o777 we should use the builtin stat.S_IMODE(mode) method since it has some additional functionality for different systems. As a result, I don't think we can necessarily assume that the leading 3 bytes will necessarily be unaltered.

Let me know if that's the approach you wanna take and I'll make the relevant changes as well as the other changes from your comment above. Also, if we do decide to use S_IMODE instead of & 0o777, do we wanna make that change for the existing code or just the code in #6

[JakobGM]

I guess that this choice depends on if we want users to be able to set "permission bits, plus the sticky bit, set-group-id, and set-user-id bits" or just the "permission bits", right?

Would you ever want to set the leading bits? That would require knowing the numeric ids of your user/group, and using chown and chgrp is more common and readable.

In any case, what the conclusion ends up being, the implementation should leave the leading 3 bytes alone if only the 3 minor bytes are specified, as in '777'. Forcing users to think about the leading bytes, when they don't care, is unpreferable, especially since I don't really understand the exact mechanics of it :P

I leave this decision to your better judgement, do what you think is best. I think we should reflect the API of UNIX-chmod to the user, whatever the implementation is. That makes the documentation really easy to write.

[sshashank124]

Based on reading these two resources: https://linux.die.net/man/1/chmod (everything before the Options header) and https://unix.stackexchange.com/questions/43744/what-does-gid-mean (accepted answer), the fields do have fairly important uses in certain cases. In the context of setting a default permission, it is not as much about what the user can/cannot specify in the configuration but rather about trying to preserve as much of the file metadata from the template to the target. Since this is implicit functionality, I think it is better to preserve as much of the mode metadata as we can.

At least for the default permissions, I am going to go ahead and use S_IMODE instead of & 0o777 since the user doesn't have to specify anything in this case.

For #4, I agree that specifying '777' should only affect those portions which is what chmod 777 <filename> would do anyways. We should just pass the permission argument as-is to chmod

I'll make these changes and update the corresponding docs

[JakobGM]

Alright, now I understand, stat.S_IMODE(mode) is going to be the default behaviour, and permissions: XXX is going to alter anything on top of that. That way we preserve all metadata, while still allowing specific permission changes throught the UNIX-chmod API. That definitely makes sense!

[sshashank124]

Wondering if we should use shutil.copystat or shutil.copymode as a more robust way to copy metadata

[JakobGM]

That is probably a good idea, only one drawback, the "last modified" datetime will also be copied over, and I consider that a bit missleading. Users will not understand why their file hasn't changed, even if an event has been triggered. Tools which inspect the modified date will also be confused. Although I do not know of any such tool, inotify is more common, and that will probably not break.

But I still think is more benefecial.

[sshashank124]

I believe copymode only copies the permission bits and not the timestamps

[JakobGM]

Okay, then we should definitely do it!