fix: use label as canonical name for zig_module and use it for module deps

[cerisier]

This fixes the scenario where 2 modules have the same name in the graph.
Currently even with an different import_name (which they need to have anyway), the build would fail with:

error: unable to add module 'data': already exists as 'path/to/main.zig'

For this, we use the str(ctx.label) which guarantees true canonical identifier, which we alias to name or import_name if it exists.

[cerisier]

CI fails are unrelated...

[aherrmann]

CI fails are unrelated...

The failure is related. Looking at the failing test binary and the corresponding shared library

$ readelf -d bazel-out/k8-fastbuild/bin/cc-dependencies/shared-library/add-test
...
 0x0000000000000001 (NEEDED)             Shared library: [lib@@//cc-dependencies/shared-library:add.so]
...
$ readelf -d bazel-out/k8-fastbuild/bin/cc-dependencies/shared-library/libadd.so | less
...
 0x000000000000000e (SONAME)             Library soname: [lib@@//cc-dependencies/shared-library:add.so]
...

So, the issue is that the canonical name finds its way into the "NEEDED" entry of the binary. Since that's not a valid shared library file name, it leads to an unresolved dynamic library dependency at runtime. The reason it ends up in the "NEEDED" entry is that it also ends up in the "SONAME" of the shared library.

The reason for that seems to be that this canonical name is used as the root module name for the library target:

$ bazel aquery //cc-dependencies/shared-library:add
...
    '-M@@//cc-dependencies/shared-library:add=cc-dependencies/shared-library/add.zig' \
...

And IIUC Zig uses that as the soname by default.

This brings up two points:

1. The default soname for shared library targets must be a valid file name and must match the file name of the generated shared object file.
2. It's interesting that Zig accepts a canonical module name that contains @/: characters without throwing an error. But, I'm not sure if there's a spec that would in principle restrict the set of characters. Even if not, it seems not unlikely that the canonical module name might be used in other filename like positions, not just the default soname. So, to be more future proof, the canonical module name should probably be sanitized to be a valid filename. Given that we're in Bazel-land, using Bazel's name mangling scheme would seem like a reasonable option.

[cerisier]

Thanks a lot for the review. That's a very good find...

I'll sanitize this!

[aherrmann]

Thank you, that looks good!
Only one comment about the translate_c bit.

[aherrmann]

I think this carries the risk of further collisions. The following example should trigger one:

zig_c_library(name = "a", import_name = "b_Uc")  # --> canonical_name = "_S_S_Ca_Ub_Uc"
zig_c_library(name = "a_b", import_name = "c")  # --> canonical_name = "_S_S_Ca_Ub_Uc"

Yes, it's contrived. But, I think a small change can avoid it.
Also, in some cases the name parameter is derived from the target name and these support a surprisingly wide set of characters including !%-@^_"#$&'()*-+,;<=>?[]{|}~/.. So, I think that part should also be escaped.

How about the following?

canonical_name = escape_label(label = "{}:{}".format(ctx.label, name))

Colon (:) is not allowed in label names and since the full string is escaped we should be safe against collisions.