libponyc: resolve relative paths in use "path:..." statements #2964
Conversation
srenatus
force-pushed
the
sr/issue-2936/resolve-use-path-relative-paths
branch
2 times, most recently
from
6baf622 to
da0ec3f
Compare
|
I'll have to update the tests. Looks like the basic setup done there doesn't create a Do y'all think this is a good approach at all? Maybe my interpretion of #2936 is off... |
|
I'm struggling with the tests a bit. I can't seem to figure out how to wrap the TK_PROGRAM into a TK_PACKAGE in such a way that the changed |
srenatus
force-pushed
the
sr/issue-2936/resolve-use-path-relative-paths
branch
from
da0ec3f to
a5ccd6d
Compare
|
💭 I've taken one step back with a5ccd6d -- it seems like the change would be more in line with how this has worked before if the relative paths are merely appended to the package's path, and not checked for existence. What do you think? 😃 |
|
@mfelsche thanks for helping with the tests! I've pushed another quick commit, attempting to fix this on windows... 🤞 |
|
@srenatus i would suggest to postpone verifying path existence in a separate PR and merge this as is. What do you think? |
mfelsche
added
the
changelog - fixed
Yeah! 😃 |
src/libponyc/pkg/program.c
Outdated
| if(strlen(locator) > FILENAME_MAX) | ||
| return false; | ||
|
|
||
| strcpy(absolute, locator); |
There was a problem hiding this comment.
i'd prefer memcpy or strncpy here.
src/libponyc/pkg/program.c
Outdated
| if(strlen(locator) > FILENAME_MAX) | ||
| return false; | ||
|
|
||
| strncpy(absolute, locator, FILENAME_MAX); |
There was a problem hiding this comment.
❌ I'll fix this in another commit soon. 😅
srenatus
force-pushed
the
sr/issue-2936/resolve-use-path-relative-paths
branch
from
ba466ff to
628772b
Compare
|
@mfelsche I've given this another go, and found a way to make this a bit simpler. What do you think? 😃 |
|
👍 |
The use of `pony_realpath` (which calls the POSIX `realpath` underneath) is convenient, but brings a change in behaviour: before, we have passed just about anything into `prog->libpaths`, regardless of the directory's existence. With `realpath`, nonexistent directories will make the call return an error. To remediate the impact a bit, I've added an `errorf()` statement. However, to make this round, I think we might have to either check for existence in both cases, or don't do that in either one. Signed-off-by: Stephan Renatus <srenatus@chef.io>
Signed-off-by: Stephan Renatus <srenatus@chef.io>
Signed-off-by: Stephan Renatus <srenatus@chef.io>
by creating a fake package.
Signed-off-by: Stephan Renatus <srenatus@chef.io>
Now, this is relying on path_cat to do copying (including length checks). Signed-off-by: Stephan Renatus <srenatus@chef.io>
srenatus
force-pushed
the
sr/issue-2936/resolve-use-path-relative-paths
branch
from
628772b to
70bf2c8
Compare
|
No idea what happened on travis. Rebased for good measure, let's see what happens now. |
This is for #2936.
The use of
pony_realpath(which calls the POSIXrealpathunderneath)is convenient, but brings a change in behaviour: before, we have passed
just about anything into
prog->libpaths, regardless of the directory'sexistence. With
realpath, nonexistent directories will make the callreturn an error.
To remediate the impact a bit, I've added an
errorf()statement.However, to make this round, I think we might have to either check for
existence in both cases~, or don't do that in either one.~
Update: I've rearrange stuff so that both absolute and relative paths are check (by being passed through
pony_realpath. This is a potentially a backwards-incompatible change; I'd think it might be one we can accept, though? Let's discuss this 😃