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

Generate user-facing errors for long filenames #8758

Closed
bradcray opened this issue Mar 9, 2018 · 11 comments · Fixed by #17559
Closed

Generate user-facing errors for long filenames #8758

bradcray opened this issue Mar 9, 2018 · 11 comments · Fixed by #17559

Comments

@bradcray
Copy link
Member

bradcray commented Mar 9, 2018

On PR #8733, @cassella writes:

if I echo 'writeln("Hello");' > $(perl -e 'print "a"x250').chpl,
that file can't be compiled because the compiler
can't create /tmp/chpl-fortytwo-26146.deleteme/aaa...aaa.tmp.o: File name too long
.tmp.o is one byte longer than .chpl.

As I understand it, this points out that our current Makefiles don't ensure that they always use filenames <= the OS max filename length and could be a problem for users who prefer using truly long filenames. On a rainy day, we could fix this.

[edit: In PR #12770, an initial attempt was made at this, but it seemed to be non-portable based on the back-end compiler used—and more specifically, the lengths of the intermediate filenames it created]

@AnubhavUjjawal
Copy link
Member

@bradcray I tried to find this error in the repo files but couldn't find it. Could you give a small idea where it might be/ where I should look?

@bradcray
Copy link
Member Author

Hi @AnubhavUjjawal: Sorry for the slow response. I don't believe we ever filed a test of this in the repo. To reproduce, you'd need to follow the instructions above from a *nix environment (where I'm assuming $ is the shell prompt below and <TAB> implies you should use TAB-completion to get the filename to expand:

$ echo 'writeln("Hello");' > $(perl -e 'print "a"x250').chpl
$ chpl aaa<TAB>.chpl

As I understand it, the issue is that we strip the .chpl extension and create other files with the same base filename (like .tmp.o) which result in a longer filename, and one that exceeds the OS limit.

Some potential fixes would be to truncate a few more characters from the filename to get within the OS limit (though this could be dangerous if there were other similarly-named files being created?), to try to avoid creating intermediate filenames with longer names, or to have the Chapel compiler complain when using a filename that was long enough that it would cause these kinds of problems. I don't have a strong preference between these options.

@cassella
Copy link
Contributor

I guess you couldn't create a normal future for this, because the name of the .future file would necessarily be too long...

@AnubhavUjjawal
Copy link
Member

@bradcray @cassella we could do this ?

Chapel compiler complain when using a filename that was long enough that it would cause these kinds of problems.

@bradcray
Copy link
Member Author

I'm definitely open to that approach.

@dgarvit

This comment has been minimized.

@king-11
Copy link
Contributor

king-11 commented Mar 16, 2021

@bradcray I would like to take up this issue. Are there any new insight that you can provide me, what I understood is

@bradcray
Copy link
Member Author

@king-11 : Moving the error to parse-time matches my thinking. As far as what to use for a limit... This is an area where I'd be inclined to do some research (what limits do most practically used OSes set in practice?) and then set it to something that feels like a conservative choice based on that. As I mentioned on #12770, I don't think there's a strong need to support super-long filenames necessarily... (and/or, we can wait for real users to want it). For me this issue is more about creating a useful error message first and foremost, and then making the limit as large as possible secondarily.

(FWIW, I called this a "rainy day project" because I don't think we've ever had a real user bump into it. @cassella tends to be good about focusing on the limits of things, which is why I think this originally came up).

You may also want to see #8757 and related PRs which aren't directly related, but maybe somewhat thematically related.

@king-11
Copy link
Contributor

king-11 commented Apr 10, 2021

@bradcray am not much familiar with parsing 😅 that's why took some time. So should the error be thrown in the following parser

static void parseCommandLineFiles() {
or if am not looking at the right place 😓 could you please help me out a bit.

@king-11
Copy link
Contributor

king-11 commented Apr 11, 2021

  • macOS: The maximum number of characters for a file name is 255.
  • Linux: The maximum length for a file name is 255 bytes.
  • Windows: the file name itself cannot exceed 255 bytes.

Also as the Unicode representation of a character can occupy several bytes, so the maximum number of characters that a file name might contain can vary. A single Unicode character can occupy as much as 32 bits | 4 bytes so considering this we should limits ourselves to somewhere around (255/4) length if we want to cover all the cases ( ? ) else somewhere around (255/2) should work well for most cases along with a reduction of 15 more character for generation of files like .tmp.o etc. 15 specifically because it seems like a good assumption 1111.

@bradcray
Copy link
Member Author

@king-11: I posted some comments on your PR. Sorry again for the delay, it's been a crazy month.

Also as the Unicode representation of a character can occupy several bytes, so the maximum number of characters that a file name might contain can vary. A single Unicode character can occupy as much as 32 bits | 4 bytes so considering this we should limits ourselves to somewhere around (255/4) length if we want to cover all the cases

I don't think this should be an issue, as I think the strlen()-style call that you're using in your PR will count the number of bytes in the filename, not the number of logical characters/glyphs/codepoints. Similarly, my guess is that macros like NAME_MAX are specifying a count in terms of bytes, not codepoints.

@bradcray bradcray changed the title Support long filenames Generate user-facing errors for long filenames Apr 28, 2021
bradcray added a commit that referenced this issue May 10, 2021
User-facing errors for long filenames

[contributed by @king-11, reviewed by me]

add check to parse cmd files for ensuring they are under limit
Fixes #8758
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment