Skip to content

Conversation

sdroege
Copy link
Contributor

@sdroege sdroege commented Feb 27, 2023

As meson requires source_dir!=build_dir and stores the rust-project.json inside the build directory, while software like rust-analyzer expects it at the root of the source directory, manual steps are needed for making them work together.

One option, as described in the documentation, is per project configuration. Another option, that works correctly with compile-commands.json and clangd, is to store a symlink to the file in the build directory at the root of the source directory.

As currently rust-project.json stores paths relative to the location of the file itself and rust-analyzer does not resolve symlinks, this does not work.

To solve this, store absolute paths in rust-project.json as is already done in compile_commands.json for the directory.


rust-analyzer PR to resolve symlinks: rust-lang/rust-analyzer#14168

CC @dcbaker

@sdroege sdroege requested a review from jpakkane as a code owner February 27, 2023 09:50
@sdroege
Copy link
Contributor Author

sdroege commented Feb 27, 2023

See also rust-lang/rust-analyzer#14168

@dcbaker
Copy link
Member

dcbaker commented Feb 27, 2023

the changes themselves look good to me. Thanks for looking at this, and especially taking the issue upstream :)

I (unfortunately, because I find pathlib to much nicer than os.path), also find it to have an unfortunate set of bugs and problematic behaviors that make it not actually as useful as it should be.

With the pathlib -> os.path changes this is r-b from me

@dcbaker dcbaker added this to the 1.0.2 milestone Feb 27, 2023
@codecov
Copy link

codecov bot commented Feb 27, 2023

Codecov Report

Merging #11467 (0bfd4e6) into master (b80f845) will increase coverage by 1.70%.
The diff coverage is n/a.

❗ Current head 0bfd4e6 differs from pull request most recent head fcefc36. Consider uploading reports for the commit fcefc36 to get more accurate results

@@            Coverage Diff             @@
##           master   #11467      +/-   ##
==========================================
+ Coverage   67.08%   68.79%   +1.70%     
==========================================
  Files         418      209     -209     
  Lines       90828    45480   -45348     
  Branches    20177     9415   -10762     
==========================================
- Hits        60936    31289   -29647     
+ Misses      25209    11775   -13434     
+ Partials     4683     2416    -2267     
Impacted Files Coverage Δ
interpreterbase/interpreterbase.py 72.96% <0.00%> (-4.64%) ⬇️
msetup.py 89.21% <0.00%> (-0.84%) ⬇️
mesonbuild/backend/ninjabackend.py
mesonbuild/compilers/compilers.py
mesonbuild/compilers/d.py
mesonbuild/compilers/mixins/clike.py
mesonbuild/coredata.py
mesonbuild/dependencies/python.py
mesonbuild/interpreter/compiler.py
mesonbuild/interpreterbase/interpreterbase.py
... and 207 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@nirbheek
Copy link
Member

I (unfortunately, because I find pathlib to much nicer than os.path), also find it to have an unfortunate set of bugs and problematic behaviors that make it not actually as useful as it should be.

Completely agree. I was all aboard the pathlib train till I started to actually use it, and now I'm fully off it.

@sdroege sdroege force-pushed the rust-project-json-absolute-paths branch from 43db0f1 to 3152685 Compare February 28, 2023 08:22
@sdroege
Copy link
Contributor Author

sdroege commented Feb 28, 2023

Updated but don't merge this yet. The problem right now is that if you run meson from inside the build directory it writes the correct path, if you run it from the source directory it won't (and will put a path relative to the source directory).

@sdroege
Copy link
Contributor Author

sdroege commented Feb 28, 2023

I think this should be fixed directly in generate_rust_target instead.

@sdroege sdroege force-pushed the rust-project-json-absolute-paths branch from 3152685 to 76d4e61 Compare February 28, 2023 13:23
@sdroege
Copy link
Contributor Author

sdroege commented Feb 28, 2023

This should work fine. For the proc macro dylib path this should be unnecessary as it seems to generate an absolute path already.

@sdroege sdroege force-pushed the rust-project-json-absolute-paths branch from e4d6ab7 to c0e8ba6 Compare February 28, 2023 14:01
@sdroege
Copy link
Contributor Author

sdroege commented Feb 28, 2023

Also fixed the proc-macro handling there in another commit. That was completely broken but now the test cases/rust/18 proc-macro/ testcase works correctly.

@sdroege
Copy link
Contributor Author

sdroege commented Feb 28, 2023

test cases/rust/11 generated main needs fixing

As meson requires source_dir!=build_dir and stores the rust-project.json
inside the build directory, while software like rust-analyzer expects it
at the root of the source directory, manual steps are needed for making
them work together.

One option, as described in the documentation, is per project
configuration. Another option, that works correctly with
compile-commands.json and clangd, is to store a symlink to the file in
the build directory at the root of the source directory.

As currently rust-project.json stores paths relative to the location of
the file itself and rust-analyzer does not resolve symlinks, this does
not work.

To solve this, store absolute paths in rust-project.json as is already
done in compile_commands.json for the directory.
The proc-macro code was not running at all because of a missing dash in
the crate type, and the proc macro dylib path was not generated as a
path but including the `-o ` commandline parameter prefix.
@sdroege sdroege force-pushed the rust-project-json-absolute-paths branch from c0e8ba6 to fcefc36 Compare February 28, 2023 15:30
@sdroege
Copy link
Contributor Author

sdroege commented Feb 28, 2023

Works here now but for some of the test cases rust-analyzer doesn't find dependencies (before and after my changes).

@nirbheek nirbheek merged commit f5841cb into mesonbuild:master Mar 1, 2023
@sdroege sdroege deleted the rust-project-json-absolute-paths branch March 1, 2023 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants