Feature/general improvements #2
Conversation
rebar.config
Outdated
| {deps, [ | ||
| {cargo, ".*", | ||
| {git, "https://github.com/lrascao/erlang-cargo.git", | ||
| {branch, "feature/target_dir"}}} |
There was a problem hiding this comment.
this branch is perched on top of rusterlium/erlang-cargo#6 and will be PR'd after it
lrascao
force-pushed
the
feature/general_improvements
branch
3 times, most recently
from
7c31cfb to
e170fa3
Compare
Through a `rebar.config` cargo opt, ie.
``
{cargo_opts, [
{src_dir, "rs_src"}
]}.
``
lrascao
force-pushed
the
feature/general_improvements
branch
from
e170fa3 to
090a867
Compare
lrascao
force-pushed
the
feature/general_improvements
branch
from
090a867 to
51a3b07
Compare
|
Ah, sorry, I didn't see the part about you waiting for a Hex release, will do that tomorrow. |
|
I guess we can wait until this one passes review |
|
i'll try and squeeze in some time this week so it can promoted out of WIP and start the review |
lrascao
force-pushed
the
feature/general_improvements
branch
from
51a3b07 to
a9ba1d0
Compare
src/rebar3_cargo_compile_prv.erl
Outdated
| IncludeDir = filename:join(OutDir0, "include"), | ||
| SrcDir = filename:join(OutDir0, "src"), | ||
| % prefer the include dir to generate crates.hrl to (if it exists) | ||
| OutDir = case filelib:is_dir(IncludeDir) of |
There was a problem hiding this comment.
No, the crates.hrl is not a "public" header file.
There was a problem hiding this comment.
agree, and it should be git ignored by any project using it. But if it's getting generated somewhere might as well be include (if possible), tipically only .erl files live in src
There was a problem hiding this comment.
The convention seems to be to have internal headers in src.
There was a problem hiding this comment.
ok, i can move this commit out of this PR context, another way would be to introduce an option for the target dir of crates.hrl that defaults to src, thoughts?
There was a problem hiding this comment.
I still stand by this, please move this change out.
There was a problem hiding this comment.
7dad8595 has been removed
There was a problem hiding this comment.
1e489afe that contained this change has been dropped
lrascao
force-pushed
the
feature/general_improvements
branch
from
7deafce to
4288306
Compare
|
@filmor just checking, is something blocking this or just not enough bandwidth atm? |
lrascao
force-pushed
the
feature/general_improvements
branch
from
4288306 to
fe7f835
Compare
Only have it return the `priv` and callers can add whatever.
The VM only cares about .dll or .so NIF's.
cargo is already generating a lot of distracting output, humans only usually care about output when something goes wrong.
On MacOS debug symbol symlink to dirs are also part of the generated artifacts, we're not interested in copying those.
We can't have a test for both (at least on MacOS), when they are both in a single Cargo workspace they will share the same dependencies. This means that when linking the .dylib it will also pick up the other binary as a dependency and now it's no longer a dynamic library.
Such as the executable permissions of the port binaries.
lrascao
force-pushed
the
feature/general_improvements
branch
from
fe7f835 to
959bd6b
Compare
|
Sorry for the lag, looks good, I'll go over it once more on the weekend and merge. |
This set of commits are general quality of life minor improvements, tests were failing on MacOS and should be passing now.
This is a WIP as it's waiting on rusterlium/erlang-cargo#6, a second PR after that one and a Hex version cut.