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

Better default resource dir heuristics: use system resource dir with … #137

Merged
merged 1 commit into from Dec 14, 2017

Conversation

Projects
None yet
3 participants
@jiegec
Contributor

jiegec commented Dec 14, 2017

…system clang and relative path to resource dir with bundled clang.

See issue #135 and #136 for the reason behind this PR.

Windows and Linux platforms are not tested. My computer is macOS.

Better default resource dir heuristics: use system resource dir with …
…system clang and relative path to resource dir with bundled clang
elif sys.platform == 'darwin':
clang_tarball_name = os.path.basename(os.path.dirname(bld.env['LIBPATH_clang'][0]))
rpath = '@loader_path/../lib/' + clang_tarball_name + '/lib'
default_resource_directory = '../lib/' + clang_tarball_name + '/resource-dir'

This comment has been minimized.

@scturtle

scturtle Dec 14, 2017

Contributor

Should ../lib/ be build/? Why not call the clang in the unzipped folder like above? For me it works on my linux.

@scturtle

scturtle Dec 14, 2017

Contributor

Should ../lib/ be build/? Why not call the clang in the unzipped folder like above? For me it works on my linux.

This comment has been minimized.

@jiegec

jiegec Dec 14, 2017

Contributor

Because, the resource dir is relative to cquery executable, and we allow cquery along with bundled clang to be installed to $PREFIX, so we have to save the relative path into cquery and let it find the correct path in runtime. Besides, it is good for reproducible build.

@jiegec

jiegec Dec 14, 2017

Contributor

Because, the resource dir is relative to cquery executable, and we allow cquery along with bundled clang to be installed to $PREFIX, so we have to save the relative path into cquery and let it find the correct path in runtime. Besides, it is good for reproducible build.

This comment has been minimized.

@jiegec

jiegec Dec 14, 2017

Contributor

rpath is the first one where loader find the dynamic libraries upon program start and it is built into the executable. $ORIGIN refers to the directory containing the program in linux, and the corresponding one in macOS is @loader_path .

@jiegec

jiegec Dec 14, 2017

Contributor

rpath is the first one where loader find the dynamic libraries upon program start and it is built into the executable. $ORIGIN refers to the directory containing the program in linux, and the corresponding one in macOS is @loader_path .

This comment has been minimized.

@jiegec

jiegec Dec 14, 2017

Contributor

When using system clang, the dynamic loader can always find clang in system directories (e.g. /usr/local/lib), thus we just want to make the resource dir of system clang to be default of the one used in cquery.
When using bundled clang, we want to force cquery to use the bundled clang and bundled resource dir. This is what this PR is all about.

@jiegec

jiegec Dec 14, 2017

Contributor

When using system clang, the dynamic loader can always find clang in system directories (e.g. /usr/local/lib), thus we just want to make the resource dir of system clang to be default of the one used in cquery.
When using bundled clang, we want to force cquery to use the bundled clang and bundled resource dir. This is what this PR is all about.

This comment has been minimized.

@scturtle

scturtle Dec 14, 2017

Contributor

I see. Thanks for your explanation!

@scturtle

scturtle Dec 14, 2017

Contributor

I see. Thanks for your explanation!

@@ -86,12 +86,15 @@ struct InitializeHandler : BaseMessageHandler<Ipc_InitializeRequest> {
// Ensure there is a resource directory.
if (config->resourceDirectory.empty()) {

This comment has been minimized.

@scturtle

scturtle Dec 14, 2017

Contributor

This confused me. For my situation, the emacs plugin always points resource-dir to the working directory in initialization options and prevent the usage of the default one. It also seems that ".." need not to be concerned on my mac or linux. I think os.path.abspath can be used in wscript.

@scturtle

scturtle Dec 14, 2017

Contributor

This confused me. For my situation, the emacs plugin always points resource-dir to the working directory in initialization options and prevent the usage of the default one. It also seems that ".." need not to be concerned on my mac or linux. I think os.path.abspath can be used in wscript.

This comment has been minimized.

@jiegec

jiegec Dec 14, 2017

Contributor

The emacs plugin needs to change accordingly (as I have done in my local cquery.el, but not in this pull request). This pull request is intented to give a resource dir that just works most of the time so users need not worry about setting it in Emacs/VS Code etc.

@jiegec

jiegec Dec 14, 2017

Contributor

The emacs plugin needs to change accordingly (as I have done in my local cquery.el, but not in this pull request). This pull request is intented to give a resource dir that just works most of the time so users need not worry about setting it in Emacs/VS Code etc.

@MaskRay

This comment has been minimized.

Show comment
Hide comment
@MaskRay

MaskRay Dec 14, 2017

Member

Looks great to me

Member

MaskRay commented Dec 14, 2017

Looks great to me

@MaskRay MaskRay merged commit f83dab8 into cquery-project:master Dec 14, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@MaskRay

This comment has been minimized.

Show comment
Hide comment
@MaskRay

MaskRay Dec 15, 2017

Member

Regarding to get executable path.

knng300 mentioned /proc is not mounted on FreeBSD by default. As an alternative,
kvm_getprocs can be used

https://www.freebsd.org/cgi/man.cgi?query=kvm_getprocs&apropos=0&sektion=3&manpath=FreeBSD+12-current&arch=default&format=html

Member

MaskRay commented Dec 15, 2017

Regarding to get executable path.

knng300 mentioned /proc is not mounted on FreeBSD by default. As an alternative,
kvm_getprocs can be used

https://www.freebsd.org/cgi/man.cgi?query=kvm_getprocs&apropos=0&sektion=3&manpath=FreeBSD+12-current&arch=default&format=html

@jiegec

This comment has been minimized.

Show comment
Hide comment
@jiegec

jiegec Dec 15, 2017

Contributor

Yes, I found some other ways for freebsd in stackoverflow. Yet I have no freebsd and there seems no travis image as well.

Contributor

jiegec commented Dec 15, 2017

Yes, I found some other ways for freebsd in stackoverflow. Yet I have no freebsd and there seems no travis image as well.

MaskRay added a commit that referenced this pull request Dec 31, 2017

Better default resource dir heuristics: use system resource dir with …
…system clang and relative path to resource dir with bundled clang (#137)

MaskRay added a commit that referenced this pull request Dec 31, 2017

Better default resource dir heuristics: use system resource dir with …
…system clang and relative path to resource dir with bundled clang (#137)

MaskRay added a commit that referenced this pull request Dec 31, 2017

Better default resource dir heuristics: use system resource dir with …
…system clang and relative path to resource dir with bundled clang (#137)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment