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

Stick to Windows-friendly filenames #4627

Closed
kmcallister opened this issue Jan 14, 2015 · 15 comments
Closed

Stick to Windows-friendly filenames #4627

kmcallister opened this issue Jan 14, 2015 · 15 comments

Comments

@kmcallister
Copy link
Contributor

@kmcallister kmcallister commented Jan 14, 2015

e.g. The question mark in hello_a?foo#bar.html is reported to cause problems. We also had issues with aux.rs in the past.

We should figure out the exact rules, then enforce them in tidy.py.

@jdm
Copy link
Member

@jdm jdm commented Jan 14, 2015

Hmm. The question mark is useful because it allows for an automated test using a query string. Maybe we just need a test harness annotation instead.

@Dexterp37
Copy link

@Dexterp37 Dexterp37 commented Feb 7, 2015

This makes git clone on Windows report the following error when checking out:

error: Invalid path 'tests/ref/hello_a?foo#bar.html'

The following article talks about allowed characters in Windows paths: link.

To recap:

  • < > : " / \ | ? *
  • Integer value zero, sometimes referred to as the ASCII NUL character.
  • Characters whose integer representations are 1-31 (less than ASCII space)
  • Any other character that the target file system does not allow (leading and trailing periods or spaces)
  • Any of the DOS names: CON, PRN, AUX, NUL, COM1, COM2, COM3, COM4, COM5, COM6, COM7, COM8, COM9, LPT1, LPT2, LPT3, LPT4, LPT5, LPT6, LPT7, LPT8, LPT9 (and avoid AUX.txt, etc)
  • The file name is all periods

Fixing this will also fix issue #3911.

@jdm
Copy link
Member

@jdm jdm commented Feb 8, 2015

I guess we should just take the "#bar" out of the filename and update the entry in basic.lit to match.

@Dexterp37
Copy link

@Dexterp37 Dexterp37 commented Feb 8, 2015

Well, the whole "?foo#bar.html" needs to be taken out because of the question mark.

@jdm
Copy link
Member

@jdm jdm commented Feb 8, 2015

Ah right. I guess we don't get to have an automated test for #3340 :(

@Manishearth
Copy link
Member

@Manishearth Manishearth commented Feb 8, 2015

We can have mach create the file based on your OS :)

@Manishearth
Copy link
Member

@Manishearth Manishearth commented Feb 8, 2015

Wait, we'd still need special harness code to make that work. Hmm.

@Dexterp37
Copy link

@Dexterp37 Dexterp37 commented Feb 8, 2015

I just noticed "%" is allowed on Windows. If that's possible on other platforms as well, maybe we could encode special characters like done with the URLs? Like %3F for "?"?

@Manishearth
Copy link
Member

@Manishearth Manishearth commented Feb 8, 2015

But the test is explicitly for testing the opening of a file with a ? in its name :)

@Dexterp37
Copy link

@Dexterp37 Dexterp37 commented Feb 9, 2015

Ouch! Then I guess conditionally creating it depending on the OS is the best bet :)

@SimonSapin
Copy link
Member

@SimonSapin SimonSapin commented Feb 9, 2015

Though it’s not great, I’d say it’s acceptable to remove ? and name the file hello_a#foo.html or something, as long as it includes #. Both ? and # are special in URLs, and this tests is about testing that a command line argument is not treated as an URL if it looks relative. So it’s unlikely we’d have this bug for one of the two characters but not both.

@frewsxcv
Copy link
Member

@frewsxcv frewsxcv commented Mar 22, 2015

I don't know how the basic.list parsing works, but what about creating a way to have only certain tests run on unix or windows?

@Manishearth
Copy link
Member

@Manishearth Manishearth commented Mar 22, 2015

You could perhaps filter filenames for nowindows, but it still makes the repo uncloneable (? not sure)

@akavel
Copy link

@akavel akavel commented Apr 3, 2015

Sorry, didn't explore thoroughly and don't know the test harness, but @Manishearth do you mean it would be hard to generate the file for a test only on nowindows OS? Could you possibly give some hints for a noob what work it could require and how to try starting to do it?

@jdm
Copy link
Member

@jdm jdm commented Sep 12, 2015

Fixed by #7610.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
7 participants
You can’t perform that action at this time.