Added pure C runfiles implementation - #849
Conversation
|
That's a good idea :) I'll battle test that in existing projects with cross-compilation to help validate. |
cerisier
left a comment
cerisier
left a comment
There was a problem hiding this comment.
A few comments from early review.
| // | ||
| // The current instance remains valid. | ||
| std::unique_ptr<Runfiles> WithSourceRepository( | ||
| const std::string& source_repository) const { |
There was a problem hiding this comment.
Is this intentional ?
This removes it entirely from the public API. And I see that tests changes below stopped exercising the method so it hid the breakage.
This should have its own C counter part.
There was a problem hiding this comment.
No, this was not intentional, restored!
| // returns <=0 for invalid paths, unknown runfiles, or buffer-too- | ||
| // small; all of those legitimately produce the empty string on the | ||
| // C++ side. | ||
| char buf[8192]; |
There was a problem hiding this comment.
So this is not true on windows where paths can be 32k.
So either respect that or make a growing equivalent of std::string ?
There was a problem hiding this comment.
This lead to a larger refactor but directionally, the C interface aims to defer how this should be handled to users.
| const char* source_repo, char* err, int err_len) { | ||
| if (!alloc) alloc = &g_libc_allocator; | ||
|
|
||
| char resolved_manifest[4096]; |
|
|
||
| int main(int argc, char** argv) { | ||
| char err[256]; | ||
| rf_runfiles* rf = rf_create(argv[0], err, sizeof(err)); |
There was a problem hiding this comment.
Clanker found that the examples do not match the API anymore.
like rf_creates requires a rf_allocator but here not.
| | C++ | `@rules_cc//cc/runfiles:runfiles` | `rules_cc/cc/runfiles/runfiles.h` | | ||
| | Pure C | `@rules_cc//cc/runfiles:runfiles_c` | `rules_cc/cc/runfiles/runfiles_c.h` | | ||
|
|
||
| The C library exposes a pluggable allocator (`rf_set_allocator`) so C |
There was a problem hiding this comment.
Stale documentation, this API doesn't exist.
| const char* dir_env_key, | ||
| const char* source_repo, char* err, | ||
| int err_len) { | ||
| char* mf = rf_getenv_alloc("RUNFILES_MANIFEST_FILE"); |
3 participants
This change re-implements the backbone of the runfiles library in C from C++ and introduces a new top level C interface at
@rules_cc//cc/runfiles:runfiles_c. Documentation for the interfaces and high level differences between the C and C++ interfaces have been added to./docs/runfiles.mdcloses #400