#4 Fix crash on member_defaults[package.name] in cargo2rpm-0.1.2-1.fc37.noarch
Closed 2 years ago by decathorpe. Opened 2 years ago by kiilerix.
fedora-rust/ kiilerix/cargo2rpm main  into  main

file modified
+1 -2
@@ -602,8 +602,7 @@ 

                          features.add(feature)

                  member_features[package.name] = features

  

-             if flags.no_default_features:

-                 member_defaults[package.name] = False

+             member_defaults[package.name] = False

  

              # ensure that the mapping includes data for all packages

              features = member_features.get(package.name) or set()

Fooling around with packaging sapling, I got:

  • /usr/bin/cargo2rpm --path Cargo.toml buildrequires
    Traceback (most recent call last):
    File "/usr/bin/cargo2rpm", line 8, in <module>
    sys.exit(main())
    ^^^^^^
    File "/usr/lib/python3.11/site-packages/cargo2rpm/main.py", line 99, in main
    action_buildrequires(args)
    File "/usr/lib/python3.11/site-packages/cargo2rpm/main.py", line 50, in action_buildrequires
    brs = workspace_buildrequires(metadata, flags, args.with_check)
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/usr/lib/python3.11/site-packages/cargo2rpm/rpm.py", line 221, in workspace_buildrequires
    member_flags = metadata.get_feature_flags_for_workspace_members(flags)
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/usr/lib/python3.11/site-packages/cargo2rpm/metadata.py", line 642, in get_feature_flags_for_workspace_members
    if member_defaults[package.name] and "default" in package.get_feature_names():
    ~~~^^^^^^^^^^^^^^
    KeyError: 'backingstore'

I don't know how special the case is that triggers this problem, but no
amount of garbage input should be able to make the tool crash. It
seems like a case that could be handled.

This could be resolved by never using indexing when looking up in
member_defaults, but always use .get() . There would thus not be any
need for initializing it to False, neither with nor without
flags.no_default_features . But it seems like it would be
more close to the intention to unconditionally initialize it to False
for all packages.

This change fixes the crash, but I am not sure it tool does the right
thing, or if the crash indicates a TODO situation.

Weird, though not unexpected. The workspace support is very new and not used by an package in Fedora yet.

Can you point me to the source for this project? I'd like to add it as a test case.

That was when hacking on packaging sapling - pretty much https://gist.github.com/kiilerix/b32cb9688d2d6e9e752e91e2ca4c30e6 on https://github.com/facebook/sapling/ . But lots of things going on there, many problems and hacks.

Ah. Looks like the Cargo.toml files are generated on-the-fly during the build. That's not going to be nice to work with :(

I haven't noticed any generated Cargo.toml , but the static ones have more than enough challenges.

Ah, I just didn't find them at first. they are under eden/scm ...

Especially things like pointing cargo at random commits in custom forks of crates is not going to work at all for RPM builds I'm afraid (well, unless you vendor those forked versions) ...

Indeed. That is something upstream will have to fix if they want their project to see general use, rather than being seen as throwing garbage over the fence ;-) But I think we all share an interest in making it work.

Yeah. I think salimma and dcavalca were looking into packaging sapling for Fedora proper, but if I remember correctly, they were waiting for some internal changes that haven't been published as OSS yet.

Anyway, I believe I have fixed this crash "properly" in commit https://pagure.io/fedora-rust/cargo2rpm/c/ee352a3c4c8287f4014854b36ebd76da94d075fc?branch=main

Thanks for the report!

Pull-Request has been closed by decathorpe

2 years ago
Metadata