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

Support server paths #1136

Merged
merged 3 commits into from
Jul 19, 2018
Merged

Support server paths #1136

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7089044
Support server paths in various path functions
Jul 17, 2018
b618a94
Add tests for server paths
Jul 17, 2018
3fec411
Merge branch 'master' into server-paths
Gaztin Jul 18, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/host/path_getabsolute.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ void do_getabsolute(char* result, const char* value, const char* relative_to)
result[0] = '\0';
if (buffer[0] == '/') {
strcat(result, "/");
if (buffer[1] == '/') {
strcat(result, "/");
}
}

prev = NULL;
Expand Down
8 changes: 8 additions & 0 deletions src/host/path_getrelative.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,14 @@ int path_getrelative(lua_State* L)
return 1;
}

/* Relative paths within a server can't climb outside the server root.
* If the paths don't share server name, return the absolute path. */
if (src[0] == '/' && src[1] == '/' && last == 1) {
Copy link
Member

Choose a reason for hiding this comment

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

I feel that this might overrun the buffer if one were to attempt to get a relative path for "". To avoid that, this could probably be:

if (src[0] != '\0' && src[0] == '/' && src[1] == '/' && last == 1) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But isn't src[0] != '\0' && src[0] == '/' redundant? if src[0] == '/' then it's guaranteed to not be 0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Additionally, empty strings are covered in the previous check.

Copy link
Member

Choose a reason for hiding this comment

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

Haha, not really sure what I was thinking when I made that comment. Sorry about that!

dst[strlen(dst) - 1] = '\0';
lua_pushstring(L, dst);
return 1;
}

/* count remaining levels in src */
count = 0;
for (i = last + 1; src[i] != '\0'; ++i) {
Expand Down
4 changes: 2 additions & 2 deletions src/host/path_normalize.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ static void* normalize_substring(const char* str, const char* endPtr, char* writ
}
}

/* add to the result, filtering out duplicate slashes */
if (ch != '/' || last != '/') {
/* add to the result, filtering out duplicate slashes, except when they are leading slashes */
if (str == &source[1] || (ch != '/' || last != '/')) {
Copy link
Member

Choose a reason for hiding this comment

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

I think this is the line I was meant to comment on. Is source guaranteed to not be an empty string here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without testing, I believe so. if the string is empty, then str will be equal to endPtr and the loop condition while (str != endPtr) will fail.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After some testing I found out that it never attempt to normalize an empty string, thanks to

premake-core/src/host/path_normalize.c

Line 108 in a713ecc

while (*endPtr) {

*(writePtr++) = ch;
}

Expand Down
16 changes: 16 additions & 0 deletions tests/base/test_path.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@
test.isequal("%HOME%/user", path.getabsolute("%HOME%/user"))
end

function suite.getabsolute_onServerPath()
test.isequal("//Server/Volume", path.getabsolute("//Server/Volume"))
end

function suite.getabsolute_onMultipleEnvVar()
test.isequal("$(HOME)/$(USER)", path.getabsolute("$(HOME)/$(USER)"))
end
Expand Down Expand Up @@ -336,6 +340,10 @@
test.isequal("obj/debug", path.getrelative("C:/Code/Premake4", "C:/Code/Premake4/obj/debug"))
end

function suite.getrelative_ReturnsChildPath_OnServerPath()
test.isequal("../Volume", path.getrelative("//Server/Shared", "//Server/Volume"))
end

function suite.getrelative_ReturnsAbsPath_OnDifferentDriveLetters()
test.isequal("D:/Files", path.getrelative("C:/Code/Premake4", "D:/Files"))
end
Expand All @@ -348,6 +356,14 @@
test.isequal("/opt/include", path.getrelative("/home/me/src/project", "/opt/include"))
end

function suite.getrelative_ReturnsAbsPath_OnServerPath()
test.isequal("//Server/Volume", path.getrelative("C:/Files", "//Server/Volume"))
end

function suite.getrelative_ReturnsAbsPath_OnDifferentServers()
test.isequal("//Server/Volume", path.getrelative("//Computer/Users", "//Server/Volume"))
end

function suite.getrelative_ignoresExtraSlashes2()
test.isequal("..", path.getrelative("/a//b/c","/a/b"))
end
Expand Down