#7 Assertion Failure when encountering path dependency in Cargo.toml
Closed 9 months ago by decathorpe. Opened 9 months ago by mulhern.

Traceback (most recent call last):
  File "/usr/bin/cargo2rpm", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File "/usr/lib/python3.12/site-packages/cargo2rpm/__main__.py", line 121, in main
    action_requires(args)
  File "/usr/lib/python3.12/site-packages/cargo2rpm/__main__.py", line 69, in action_requires
    items = requires(metadata.packages[0], feature)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/cargo2rpm/rpm.py", line 414, in requires
    return _requires_crate(package)
           ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/cargo2rpm/rpm.py", line 292, in _requires_crate
    deps.add(dep.to_rpm("default"))
             ^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/cargo2rpm/metadata.py", line 125, in to_rpm
    assert self.path is None, "Attempt to generate an RPM dependency for a path dependency!"
AssertionError: Attempt to generate an RPM dependency for a path dependency!

This arises because we have decided that it would be better to bundle our -sys lib w/ our outer lib in our bindings packages, because maintaining them as separate packages in Fedora has become exhausting.

Therefore, there is one path dependency in the crate and cargo2rpm is suffering an assertion failure when it encounters that one path dependency. The consequence of that will be, I believe, that it will be unable to properly generate the Requires entries in the rpm, as we see it doing here: https://koji.fedoraproject.org/koji/rpminfo?rpmID=35320245.

Here's the log of the failure: https://kojipkgs.fedoraproject.org//work/tasks/8210/107038210/build.log.

I think that probably the best way to fix this is to avoid calling the to_rpm() method on a dependency with a path component, i.e., a path dependency.

I expect that this is straightforward and that if I were to submit a PR the process would take longer. But I'm happy to submit a PR if that is preferred.


Not supporting this is intentional.

The only way to build with path dependencies is if you don't build -devel subpackages.

Metadata Update from @decathorpe:
- Issue status updated to: Closed (was: Open)

9 months ago

I am at my desk now, so I can expand on my previous comment:

  • crate registries do not support (or allow) path-based dependencies, since there is - in general - no way to resolve them
  • path-based dependencies are stripped from Cargo.toml by cargo package and cargo publish, and prevent packages that fail to build without them from being published
  • our packaging is based on a crate registry (a directory-based registry in /usr/share/cargo/registry)
  • hence, crates with path-based dependencies are not allowed to be installed into %cargo_registry / %crate_instdir since those are not allowed and not supported in registries

If they were allowed, what should the dependency generator do in this case? Translating path-based dependencies into normal dependencies is not possible in isolation. But just ignoring them would lead to incomplete metadata, missing dependencies, and build failures. Hence, if they were allowed, you would just create problems later on in dependent packages.

So the assertion failure you have encountered is actually a footgun prevention mechanism.

Note that there is also this MUST requirement from the Rust packaging guidelines, which this package seems to violate:

https://docs.fedoraproject.org/en-US/packaging-guidelines/Rust/#_library_crates

Responding:

  • crate registries do not support (or allow) path-based dependencies, since there is - in general - no way to resolve them

In this particular case, the way to resolve the path-based dependency is to use the path. The source is there, and was always part of the GitHub release for the package.

  • path-based dependencies are stripped from Cargo.toml by cargo package and cargo publish, and prevent packages that fail to build without them from being published

I figured this out long ago. I made a patch to put this particular path-based dependency back in.

  • our packaging is based on a crate registry (a directory-based registry in /usr/share/cargo/registry)

Yup. I figured this out, too.

hence, crates with path-based dependencies are not allowed to be installed into %cargo_registry / %crate_instdir since those are not allowed and not supported in registries

I don't see how this is a "hence". But I do know that crates.io does not allow path-based dependencies.

If they were allowed, what should the dependency generator do in this case? Translating path-based dependencies into normal dependencies is not possible in isolation. But just ignoring them would lead to incomplete metadata, missing dependencies, and build failures. Hence, if they were allowed, you would just create problems later on in dependent packages.

I don't think it's in isolation because the source code is in the source rpm. It should skip that dependency, possibly checking to make sure the source code is actually there.

So the assertion failure you have encountered is actually a footgun prevention mechanism.

The assertion failure doesn't prevent anything except generating the correct Build Requires as the build succeeds just fine. I had to scan the build log to detect that this problem had arisen. It doesn't prevent the crate compiling, because the generate_build_requires works correctly because the source is there for it to build from.

https://docs.fedoraproject.org/en-US/packaging-guidelines/Rust/#_library_crates

This is completely orthogonal to the concern I brought up, right? I think that the package name, rust-devicemapper was chosen, correctly, to match the crate name devicemapper but that neither of these match the GitHub repo name devicemapper-rs.

You, in a recent conversation, somewhere, mentioned that some people have figured out how to do what I am trying to do, i.e., use a path dependency like this. But it looks like an actual path dependency will not work, based on our conversation. Perhaps the local registry would, ultimately, reject it. I'll see if I can come up with something else.

I think that when you mentioned path dependencies, you were talking only about packaging for executables. That would make sense, in retrospect.

So the assertion failure you have encountered is actually a footgun prevention mechanism.

The assertion failure doesn't prevent anything except generating the correct Build Requires as the build succeeds just fine. I had to scan the build log to detect that this problem had arisen. It doesn't prevent the crate compiling, because the generate_build_requires works correctly because the source is there for it to build from.

This is not intended, a failure in the RPM generator should abort the build. I need to look into why it doesn't in this case.

https://docs.fedoraproject.org/en-US/packaging-guidelines/Rust/#_library_crates

This is completely orthogonal to the concern I brought up, right? I think that the package name, rust-devicemapper was chosen, correctly, to match the crate name devicemapper but that neither of these match the GitHub repo name devicemapper-rs.

No, it's not. The rule is:

Source packages for Rust crates which contain a library with a public API MUST be named rust-$crate.

  • crate = devicemapper: source package name MUST be rust-devicemapper
  • crate is devicemapper-sys: source package name MUST be rust-devicemapper-sys

The name of the upstream project does not matter, the name of the crate as published on crates.io does.

You, in a recent conversation, somewhere, mentioned that some people have figured out how to do what I am trying to do, i.e., use a path dependency like this. But it looks like an actual path dependency will not work, based on our conversation. Perhaps the local registry would, ultimately, reject it. I'll see if I can come up with something else.

I think that when you mentioned path dependencies, you were talking only about packaging for executables. That would make sense, in retrospect.

Yes, this was for leaf packages / binaries. It will not work for library crates.


There's just so many ways for things to go wrong once we start allowing path-based dependencies in our crate registry.

For example, crates are expected to be installed at /usr/share/cargo/registry/$CRATE-$CRATE_VERSION/. Using path-based dependencies, this will break in one of two ways:

  • Option 1: "inner" crate is installed to /usr/share/cargo/registry/devicemapper-sys-$VERSION/, and "outer" crate is installed to /usr/share/cargo/registry/devicemapper-$VERSION/. In this case, the path-based dependency in the "outer" crate will fail to resolve because the paths no longer match.
  • Option 2: "inner" crate is installed inside the "outer" crate at /usr/share/cargo/registry/devicemapper-$VERSION/devicemapper-sys/. This means it is no longer visible to cargo other than via the "devicemapper" crate, and all packages depending on the "inner" crate via normal means will no longer find it.

If I spend more time on it, I can probably come up with other reasons why this is a bad idea ... so PLEASE don't do this. It is not a supported way to package library crates, it is a MUST NOT in the Packaging Guidelines for a reason, and even if you could make it work now, it will likely break in unexpected ways in the future.

Log in to comment on this ticket.

Metadata