Skip to content
This repository has been archived by the owner on Mar 10, 2020. It is now read-only.

docs: add mode and mtime and .touch/.chmod mfs commands #572

Merged
merged 12 commits into from
Jan 9, 2020

Conversation

achingbrain
Copy link
Collaborator

@achingbrain achingbrain commented Dec 12, 2019

Needs more tests adding before this is ready. Early feedback is encouraged.

SPEC/FILES.md Outdated Show resolved Hide resolved
SPEC/FILES.md Outdated
@@ -58,6 +60,8 @@ Where `data` may be:
{
path: '/tmp/myfile.txt', // The file path
content: <data> // A Buffer, Readable Stream, Pull Stream or File with the contents of the file
mode: '0755' // optional string or integer mode to store the entry with. strings will be interpreted as a base 8 number
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I think about a unix mode as a string I think “rwx” “r--“ etc. Why not use this format as our stringy mode?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah? I have to admit I don't when setting modes, only when viewing them with ls -l or whatever.

In my head at least, the choice was between a literal base 10 number (e.g. 493) or a literal base 8 number (e.g. 0755). Linters don't seem to like literal numbers in anything other than base 10 so that's why it's a string '0755' above.

It's good to be flexible though, maybe something we could add later?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it helps to get this stuff merged faster then sure 👍 We’ll be supporting rwx format in chmod so I reckon people will expect to be able to use it here too.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We support Symbolic Notation in chmod (e.g. a=rwx, ug-x, etc) which are modifications to an existing mode.

I guess if you want to use it here you could apply the modification to the default mode for files/directories?

SPEC/FILES.md Outdated Show resolved Hide resolved
SPEC/FILES.md Outdated Show resolved Hide resolved
@achingbrain achingbrain marked this pull request as ready for review December 28, 2019 18:49
Copy link
Contributor

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Documentation review.

SPEC/FILES.md Outdated Show resolved Hide resolved
SPEC/FILES.md Outdated Show resolved Hide resolved
SPEC/FILES.md Outdated Show resolved Hide resolved
SPEC/FILES.md Outdated Show resolved Hide resolved
await ipfs.files.chmod('/path/to/file.txt', parseInt('0777', 8))

// Alternatively
await ipfs.files.chmod('/path/to/file.txt', '+rwx')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?+rwx?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I follow - what does the leading ? mean in ?+rwx? I can't see any reference to it in man chmod and when I try to do this locally I get an error:

$ chmod ?+rwx foo.txt
chmod: Invalid file mode: ?+rwx

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, nevermind. I didn't realize +rwx was equivalent to a+rwx.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good point, it will support u+rwx, g+r, o+w, a-w etc. though right? We should specify what subset of https://en.wikipedia.org/wiki/Modes_(Unix)#Format we do support.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it'll support all of those.

Though now you mention it there's a testing gap for a...

..and maybe not X right now, though it can be added.

@achingbrain
Copy link
Collaborator Author

I'm going to merge this as-is since I think we're vaguely in agreement, we can address any issues in further PRs.

@achingbrain achingbrain merged commit e10e85d into master Jan 9, 2020
@achingbrain achingbrain deleted the add-unixfs-metadata-tests-and-docs branch January 9, 2020 10:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants