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.

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

mkdir: suppress multiple errors for non-directory argument #893

Merged
merged 3 commits into from
Jan 3, 2025

Conversation

mknos
Copy link
Contributor

@mknos mknos commented Jan 2, 2025

  • Previously if running "mkdir -p" and the 1st part of an argument is not a directory, the program would incorrectly call mkdir() on the intermediate paths under it
  • If I have a regular file 0, "mkdir -p 0/1/2" would show 3 errors because of mkdir("0/1") and mkdir("0/1/2")
  • Restructure the program so ARGV can remain as a list of directories (now we don't need the explanation comment for the old confusing code)
  • For -p flag, split directory into parts, calling a create_dir() function for each part
  • split_dir() handles the case where the 1st part is not a directory, and drops the sub-directory parts
%touch 0
%perl mkdir -p 0/1/2 00 aa/bb/cc # only one error for 0/1/2 argument
mkdir: cannot create directory '0': File exists

%find 00 aa # everything was created except for 0/...
00
aa
aa/bb
aa/bb/cc

* Previously if running "mkdir -p", and the 1st part of an argument is not a directory, the program would attempt to call mkdir() on the intermediate paths under it
* So if I have regular file 0, "mkdir -p 0/1/2" would show 3 errors because of mkdir("0/1") and mkdir("0/1/2")
* Restructure the program so ARGV can remain as a list of directories (now we don't need the explanation comment for the old confusing code)
* For -p flag, split directory into parts, calling a create_dir() function for each part
* split_dir() handles the case where the 1st part is not a directory, and drops the sub-directory parts

%touch 0
%perl mkdir -p 0/1/2 00 aa/bb/cc
mkdir: cannot create directory '0': File exists

%find 00 aa # everything was created except for 0/...
00
aa
aa/bb
aa/bb/cc
@github-actions github-actions bot added Type: enhancement improve a feature that already exists Priority: low get to this whenever Program: mkdir The mkdir program labels Jan 2, 2025
@mknos mknos temporarily deployed to automated_testing January 2, 2025 01:57 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing January 2, 2025 01:57 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing January 2, 2025 01:57 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing January 2, 2025 01:57 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing January 2, 2025 01:57 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing January 2, 2025 01:57 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing January 2, 2025 01:57 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing January 2, 2025 01:57 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing January 2, 2025 01:57 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing January 2, 2025 01:57 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing January 2, 2025 01:57 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing January 2, 2025 01:57 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing January 2, 2025 01:57 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing January 2, 2025 01:57 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing January 2, 2025 01:57 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing January 2, 2025 01:57 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing January 2, 2025 01:57 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing January 2, 2025 01:57 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing January 2, 2025 01:57 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing January 2, 2025 01:57 — with GitHub Actions Inactive
@coveralls
Copy link

coveralls commented Jan 2, 2025

Pull Request Test Coverage Report for Build 12578806958

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 72.464%

Totals Coverage Status
Change from base Build 12566564471: 0.0%
Covered Lines: 350
Relevant Lines: 483

💛 - Coveralls

@mknos mknos temporarily deployed to automated_testing January 2, 2025 02:04 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing January 2, 2025 02:04 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing January 2, 2025 02:04 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing January 2, 2025 02:04 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing January 2, 2025 07:05 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing January 2, 2025 07:06 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing January 2, 2025 07:06 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing January 2, 2025 07:06 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing January 2, 2025 07:06 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing January 2, 2025 07:06 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing January 2, 2025 07:06 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing January 2, 2025 07:06 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing January 2, 2025 07:06 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing January 2, 2025 07:06 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing January 2, 2025 07:06 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing January 2, 2025 07:06 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing January 2, 2025 07:06 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing January 2, 2025 07:06 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing January 2, 2025 07:06 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing January 2, 2025 07:06 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing January 2, 2025 07:06 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing January 2, 2025 07:06 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing January 2, 2025 07:06 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing January 2, 2025 07:06 — with GitHub Actions Inactive
@mknos
Copy link
Contributor Author

mknos commented Jan 2, 2025

Thanks for the suggestion. I added a new commit which rewrites split_dir() using File::Spec.
I found another test case where subdir creation should bail out: permission denied error when creating parent dir.
create_dir() now returns true on success so the subdir creation can stop on error.
This means split_dir() has no need to do any file tests.
Handle special case where File::Spec adds a leading '' list element for absolute paths.

%perl mkdir -p 0/1/2 /0/1/2 aa/bb/cc 00 # single error for 0/1/2 and /0/1/2 respectively
mkdir: cannot create directory '0': File exists
mkdir: cannot create directory '/0': Permission denied

%find aa 00
aa
aa/bb
aa/bb/cc
00

@briandfoy briandfoy merged commit e4c30c5 into briandfoy:master Jan 3, 2025
22 checks passed
@briandfoy briandfoy added Status: accepted The fix is accepted and removed Priority: low get to this whenever Status: changes requested adjust the pull request as noted in comments labels Jan 3, 2025
@briandfoy
Copy link
Owner

changes: suppress multiple errors for non-directory argument

@briandfoy briandfoy self-assigned this Jan 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Program: mkdir The mkdir program Status: accepted The fix is accepted Type: enhancement improve a feature that already exists
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants