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

Provide a directory separator if needed when converting a relative to an absolute path #293

Conversation

sethfowler
Copy link
Contributor

You can't run p4c without installing it on macOS right now. The cause is that when searching for the P4 include files, on macOS we attempt to use the approach where we treat argv[0] as a relative path, but we don't insert a directory separator. The result is that when you run ./p4c-bm2-ss, we act as if the compiler is located at a path like /Users/sfowler/p4c/build./p4c-bm2-ss, which is obviously wrong. This prevents us from finding the include files, and we can't compile anything. This patch solves this problem. The patch is agnostic as to whether getcwd() returns a path with a directory separator at the end; I'm not sure whether this is consistent across platforms.

@sethfowler sethfowler self-assigned this Feb 10, 2017
@sethfowler sethfowler changed the title Provide a directory separator if needed when converting relative to an absolute path Provide a directory separator if needed when converting a relative to an absolute path Feb 10, 2017
Copy link
Contributor

@ChrisDodd ChrisDodd left a comment

Choose a reason for hiding this comment

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

This was just inadequately tested -- I suspect getcwd always returns a path without the trailing /, though the documentation doesn't say.

Probably we should be using _NSGetExecutablePath on OSX anyways.

@sethfowler sethfowler force-pushed the seth/convert-relative-path-to-absolute-path-correctly branch from 8e6d0cd to 15ee54e Compare February 10, 2017 02:33
@sethfowler
Copy link
Contributor Author

Yeah, agreed on both counts. Might be nice to add a little header in lib/ at some point for platform-specific wrappers for this kind of case.

@sethfowler sethfowler force-pushed the seth/convert-relative-path-to-absolute-path-correctly branch from 15ee54e to 44bcb40 Compare February 10, 2017 18:30
@sethfowler sethfowler merged commit 0d4ba2b into p4lang:master Feb 10, 2017
@sethfowler sethfowler deleted the seth/convert-relative-path-to-absolute-path-correctly branch February 10, 2017 18:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants