fix os.isdir() and os.mkdir() - #2759
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
| local ok, err = os.mkdir("folder/ok.lua") | ||
| test.isnil(ok) | ||
| test.isequal("string", type(err)) | ||
| test.istrue(os.isfile("folder/ok.lua")) |
There was a problem hiding this comment.
I would move test.istrue(os.isfile("folder/ok.lua")) at the beginning. It is the prerequire for the failure of os.mkdir
| -- | ||
|
|
||
| function suite.mkdir_ReturnsTrue_OnExistingDirectoryLink() | ||
| -- BSD CI mounts the workspace through SSHFS, so keep topology tests on local storage. |
There was a problem hiding this comment.
https://github.com/premake/premake-core/actions/runs/30967252835/job/92183785538
after os.linkdir("folder/subfolder", "folder/subfolder2"), os.mkdir("folder/subfolder2") fail.
There was a problem hiding this comment.
I’m not sure. I can only guess that it has something to do with SSHFS.
There was a problem hiding this comment.
Likely related to issues that I started addressing with #2749, if I had to guess. Our BSD VM seems to be breaking quite a bit of code that works elsewhere. Not sure if it's with the VM set up or if it's BSD itself.
There was a problem hiding this comment.
The comment reads as if it's obvious why you'd use local storage when using BSD and SSHFS, but then you're not sure why it doesn't work. We might just be calling the wrong functions on BSD? It would be nice to have more investigation into the issue so that we can clearly state why this test doesn't work with that configuration.
There was a problem hiding this comment.
The comment reads as if it's obvious why you'd use local storage when using BSD and SSHFS, but then you're not sure why it doesn't work. We might just be calling the wrong functions on BSD? It would be nice to have more investigation into the issue so that we can clearly state why this test doesn't work with that configuration.
Sorry, I'm not interested about BSD. So I've just removed the test.
|
I want to follow up on this. Currently, our CI jobs are passing. Is this a gap in our current testing? |
The main issue is that previously,
|
cbc65b2 to
ad11fdd
Compare
4 participants
What does this PR do?
_wstatdoes not fully recognize reparse points (symbolic links);GetFileAttributesWneeds to be used instead.os.mkdirincorrectly returned success when the target already existed and was a file, added the test.os.isdir.How does this PR change Premake's behavior?
In the edge case where the target of
mkdiralready exists and is a file, the operation may now terminate earlier.Anything else we should know?
Did you check all the boxes?
closes #XXXXin comment to auto-close issue when PR is merged)You can now support Premake on our OpenCollective. Your contributions help us spend more time responding to requests like these!