openSUSE / libsolv

Library for solving packages and reading repositories
http://en.opensuse.org/openSUSE:Libzypp_satsolver
Other
524 stars 153 forks source link

repodata.add_dirstr() doesn't work with empty "dir", common to SRPM package "files" #397

Open dralley opened 4 years ago

dralley commented 4 years ago

This assertion: https://github.com/openSUSE/libsolv/blob/ac194e6f024420c0be72c49def6a80adc33dd4bf/src/repodata.c#L2929

Is problematic in the case of SRPMs, which frequently have files without directory. This means that dir is an empty string, which is converted to Id 0, which fails the assertion. Therefore parsing filelist data for a repository containing SRPMs or RPMs mixed with SRPMs will hit assertion failures unless action is taken externally to skip past the SRPM filelists.

See: standard RPMs have directory, SRPMs do not.

[dalley@localhost devel]$ rpm -qlp fzf-0.22.0-1.fc32.src.rpm README.Fedora fzf-0.22.0.tar.gz fzf.spec

[dalley@localhost devel]$ rpm -qlp fzf-0.22.0-1.fc32.x86_64.rpm /etc/bash_completion.d/fzf /usr/bin/fzf /usr/bin/fzf-tmux ...

This is the Python code being used to process file

    def rpm_filelist_conversion(solvable, unit):
        """A specific, rpm-unit-type filelist attribute conversion."""
        repodata = solv_repo.first_repodata()

        # file_repr = [None, '', 'test-srpm.spec']
        for file_repr in unit.get('files', []):
            dir_path = file_repr[1]    # empty string
            dirname_id = repodata.str2dir(dir_path)   # Id 0
            repodata.add_dirstr(
                solvable.id, solv.SOLVABLE_FILELIST,
                dirname_id, os.path.basename(dir_path)
            )
            # assertion failure: /tmp/pip-req-build-rneggjoc/src/repodata.c:2878: repodata_add_dirstr: Assertion `dir' failed.

The following patch successfully works around the problem and is good enough for us, but I'm curious whether there is a better way to solve it or if this is an unintentional oversight that should be fixed upstream.

         for file_repr in unit.get('files', []):
             dir_path = file_repr[1]
+            if not dir_path:
+                continue
             dirname_id = repodata.str2dir(dir_path)
             repodata.add_dirstr(

I have no idea if the methods for loading libsolv directly from repository XML are affected by this since we don't use them.

mlschroe commented 4 years ago

The code in ext (e.g. the XML parser) works around this by replacing an empty dir with '/':

         if (!did)
            did = repodata_str2dir(data, "/", 1);

The limitation is pretty artificial, though. It's not hard to remove it, so that's what we're going to do in the future.