Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Implement Unix file regularity check (#20448) #20628

Merged
merged 2 commits into from
Oct 25, 2022
Merged

Implement Unix file regularity check (#20448) #20628

merged 2 commits into from
Oct 25, 2022

Conversation

a-mr
Copy link
Contributor

@a-mr a-mr commented Oct 23, 2022

Implements #20448.

@Araq Araq requested a review from ringabout October 24, 2022 08:07
ringabout
ringabout reviewed Oct 24, 2022
View reviewed changes
Copy link
Member

@ringabout ringabout left a comment

Choose a reason for hiding this comment

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

What about walkPattern and walkFiles? There is also a new module called std/dirs which is a type safe version of std/private/osdirs which should be patched too.

@ringabout ringabout requested a review from Varriount October 24, 2022 17:03
@a-mr
Copy link
Contributor Author

a-mr commented Oct 24, 2022

A user can just combine walkPattern/walkFiles with a call to getFileInfo and checking field isRegular.

This was a conscious design decision :-). walkPattern/walkFiles use Unix glob function for implementation, which does not return info on file type. The point of this PR is in that current syscalls in walkDir/getFileInfo already return the needed information, so this regularity check can be done without any overhead. (Also note that walkPattern/walkFiles don't have options relative and checkDir implemented.) TBH my opinion is that walkPattern/walkFiles are redundant and had better be deprecated.

  • std/dirs — done

Varriount
Varriount reviewed Oct 24, 2022
View reviewed changes
lib/pure/os.nim
@@ -1095,6 +1095,8 @@ type
creationTime*: times.Time ## Time file was created. Not supported on all systems!
blockSize*: int ## Preferred I/O block size for this object.
## In some filesystems, this may vary from file to file.
isRegular*: bool ## Is file regular? (on Unix some "files"
## can be non-regular like FIFOs, devices)

template rawToFormalFileInfo(rawInfo, path, formalInfo): untyped =
Copy link
Contributor

@Varriount Varriount Oct 24, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

  • On *nix, are symlinks, etc. considered "regular files"?
  • Windows needs to be handled. If the above question is true, then isRegular can just always be set to true.
  • If it's not too much trouble, I think it's time for rawToFormalFileInfo to be split into 2 different templates (one for each platform).

Copy link
Contributor Author

@a-mr a-mr Oct 25, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

On *nix, are symlinks, etc. considered "regular files"?

Not really, they are a separate type of file (even links to directories). But here isRegular can be both true or false depending on target. I'll think about documenting that more thorougly.

Windows needs to be handled. If the above question is true, then isRegular can just always be set to true.

Thank you. May be an omission, I'll check that.

@ringabout
Copy link
Member

TBH my opinion is that walkPattern/walkFiles are redundant and had better be deprecated.

It can be removed from std/dirs which is a new module.

@ringabout ringabout mentioned this pull request Oct 25, 2022
Merged
@Araq Araq merged commit 8ed2431 into nim-lang:devel Oct 25, 2022
@github-actions
Copy link
Contributor

Thanks for your hard work on this PR!
The lines below are statistics of the Nim compiler built from 8ed2431

Hint: mm: orc; opt: speed; options: -d:release
164057 lines; 9.653s; 613.211MiB peakmem

a-mr added a commit to a-mr/Nim that referenced this pull request Oct 26, 2022
@a-mr
Fix nim-lang#20628 for Windows
7c811bf
Araq pushed a commit that referenced this pull request Oct 28, 2022
@a-mr
Fix #20628 for Windows (#20667)
779b1cc 
* Fix #20628 for Windows

* Move isRegular - !isSpecial and onlyRegular - skipSpecial

* Forgot to change it in 1 more place
@ringabout ringabout mentioned this pull request Nov 5, 2022
Closed
capocasa pushed a commit to capocasa/Nim that referenced this pull request Mar 31, 2023
@a-mr @capocasa
Implement Unix file regularity check (nim-lang#20448) (nim-lang#20628)
5ec98e0 
* Implement Unix file regularity check

* update std/dirs also
capocasa pushed a commit to capocasa/Nim that referenced this pull request Mar 31, 2023
@a-mr @capocasa
Fix nim-lang#20628 for Windows (nim-lang#20667)
dadfb0f 
* Fix nim-lang#20628 for Windows

* Move isRegular - !isSpecial and onlyRegular - skipSpecial

* Forgot to change it in 1 more place
bung87 pushed a commit to bung87/Nim that referenced this pull request Jul 29, 2023
@a-mr @bung87
Implement Unix file regularity check (nim-lang#20448) (nim-lang#20628)
f33ad7b 
* Implement Unix file regularity check

* update std/dirs also
bung87 pushed a commit to bung87/Nim that referenced this pull request Jul 29, 2023
@a-mr @bung87
Fix nim-lang#20628 for Windows (nim-lang#20667)
69da118 
* Fix nim-lang#20628 for Windows

* Move isRegular - !isSpecial and onlyRegular - skipSpecial

* Forgot to change it in 1 more place
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Varriount Varriount Varriount left review comments

@ringabout ringabout ringabout left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@a-mr @ringabout @Varriount @Araq