search

google / emboss

Emboss is a tool for generating code that reads and writes binary data structures.
Apache License 2.0
71 stars 21 forks source link

Bazel: emboss_cpp_library uses incorrect import path when in an external repository #105

Closed BenjaminLawson closed 7 months ago

BenjaminLawson commented 8 months ago

When the emboss_cpp_library rule is an an external Bazel repository, the import path that emboss_cpp_library tries to use is incorrect.

This does not work:

import "pw_bluetooth/public/pw_bluetooth/hci_common.emb" as hci

this works:

import "external/pigweed/pw_bluetooth/public/pw_bluetooth/hci_common.emb" as hci

I don't think emboss_cpp_library should require import paths to start with "external/repo-name".

BenjaminLawson commented 8 months ago

Maybe --import-dir should be set to external/repo-name instead of "." and " ".

BenjaminLawson commented 8 months ago

src.path resolves to the right path (external/repo-name/.../src), but src.root resolves to the directory containing "external" and is used for --import-dir. It seems like we want to use "src.root + /external/repo-name" to resolve imports.

BenjaminLawson commented 8 months ago

Yes, imports = ["--import-dir=" + root.path + "external/pigweed/" for root in dep_roots.to_list()] in _emboss_library_impl works. I'm not sure how to properly calculate "external/pigweed" though.

BenjaminLawson commented 8 months ago

This seems to work:

    adjusted_src_root = src.root.path
    if src.path.startswith("external/"):
        segments = src.path.split("/")
        first_two_segments = "/".join(segments[:3])
        adjusted_src_root = paths.dirname(first_two_segments)

    dep_roots = depset(
        direct = [adjusted_src_root],
        transitive = [dep.transitive_roots for dep in deps],
    )
    imports = ["--import-dir=" + root for root in dep_roots.to_list()]
reventlov commented 8 months ago

That's sort of horrible, but probably good enough for now.

It would probably be worth reaching out to the Bazel team to see if they have any suggestions -- I was basically trying to copy the aspect for Proto, but that aspect is allowlisted for a bunch of interfaces that I could not use directly.

BenjaminLawson commented 7 months ago

Sounds like proto has the same problem. I found a similar solution on stackoverflow: https://stackoverflow.com/questions/55796252/how-to-write-bazel-rules-that-work-with-external-repositories