Add optional filelists_ext metadata by aplanas · Pull Request #316 · rpm-software-management/createrepo_c

aplanas · 2022-03-24T14:13:22Z

Using the parameter '--filelists_ext', a new metadata file
(filelists_ext) will be created. This new metadata is a copy of
filelists, but will include a new attribute for the 'file' element,
named 'hash', that contain the registered hash (SHA256 for example)
for this file.

Fix #313

Signed-off-by: Alberto Planas [email protected]

aplanas · 2022-03-24T14:14:21Z

There are some bits missing, mostly tests, some adjustments in the Python bindings, and all the sqlite code.

But I want to present this patch as early as possible, to see if the approach will be OK

mlschroe · 2022-03-25T16:26:25Z

The name "--hash" is somewhat confusing. Rpm calls those "digest", so why not --digest? And why is the filename "filelists_ext" if the option is called "hash"? Shouldn't it be "filehashes" or "filedigests"?

mlschroe · 2022-03-25T16:29:21Z

No testcases?

aplanas · 2022-03-25T16:47:43Z

No testcases?

The first comment point other missing bits too. The patch was getting big without any review, so I decided to publish the code to have the chance of fixing issues before too long.

Tests and SQLIte schema change will be added in this PR before removing the draft flag.

aplanas · 2022-03-25T16:53:01Z

The name "--hash" is somewhat confusing. Rpm calls those "digest", so why not --digest?

Sure, digest is used in the internal package structure. I can use also --filelists_ext

And why is the filename "filelists_ext" if the option is called "hash"? Shouldn't it be "filehashes" or "filedigests"?

filelists_ext.xml is a strict superset of filelists.xml. For reasons of why is that see #313 (comment)

I think I will go for --filelists_ext then.

aplanas · 2022-03-28T09:00:30Z

The name "--hash" is somewhat confusing.

Done. I renamed it to --filelists_ext

kontura · 2022-03-29T13:13:10Z

Given that the filelists-ext fully contains regular filelists how about instead of adding all the new parsing APIs we just use the existing ones? I think it would just work since the element was added to xml_parser_filelists.c.

src/xml_parser_filelists.c

aplanas · 2022-03-29T13:17:59Z

Given that the filelists-ext fully contains regular filelists how about instead of adding all the new parsing APIs we just use the existing ones? I think it would just work since the element was added to xml_parser_filelists.c.

I like the idea. The parser state machine is shared, I will see how I can unify the API. I must admit that when I do not understand the code, I make an explicit copy for the filelists_ext version.

aplanas · 2022-03-29T14:54:01Z

Given that the filelists-ext fully contains regular filelists how about instead of adding all the new parsing APIs we just use the existing ones?

Ok, I think that I know how to do this, thanks for the idea as it will remove a lot of issues.

aplanas · 2022-03-30T14:10:30Z

I still plan to add some tests, but I am thinking to drop the changes in sqlite for now. Meanwhile I moved the PR to ready for reviews.

aplanas · 2022-04-01T12:39:03Z

I updated some tests, and I decided not to break the cr_PackageFiles Python tuple, and not include the digest there.

IMHO I am done.

dralley · 2022-04-10T00:06:54Z

I still plan to add some tests, but I am thinking to drop the changes in sqlite for now. Meanwhile I moved the PR to ready for reviews.

Yes, what I've now realized is that .sqlite metadata is de-facto deprecated even though it's still provided in most repos. None of the tools that consume it are mainstream anymore, except for legacy yum in el7, which has only a few years to live, and of course this new feature isn't super relevant to it.

I filed this issue to turn it off in a future Fedora release. https://pagure.io/releng/issue/10745

So no changes to the sqlite metadata is likely preferable.

dralley · 2022-04-10T02:29:47Z

src/xml_dump.c

        }
+
+        if (filelists_ext && entry->digest && entry->digest[0] != '\0') {
+            cr_xmlNewProp(file_node, BAD_CAST "hash", BAD_CAST entry->digest);


This probably should have an attribute to distinguish the digest algorithm used alongside the digest itself.

There is a new XML field for each package, that indicate the algorithm (<checksum/>), this tag is the file digest itself

dralley · 2022-04-10T02:33:04Z

tests/test_xml_parser_main_metadata_together.c

+    cr_Package *package = NULL;
+
+    cr_PkgIterator *pkg_iterator = cr_PkgIterator_new(
+        TEST_REPO_04_PRIMARY, TEST_REPO_04_FILELISTS_EXT, TEST_REPO_04_OTHER, NULL, NULL, NULL, NULL, &tmp_err);


My understanding of this API then, is that if you want the full metadata, you substitute the path to filelists_ext.xml instead of filelists.xml?

For the parser, this is the idea.

Conan-Kudo · 2022-12-07T21:16:47Z

@aplanas Are you still interested in this? It needs rebasing...

aplanas · 2022-12-08T08:27:23Z

@Conan-Kudo Yes. Rebased!

Is there anything actionable that I can do to bootstrap the discussion? Is the goal of this PR clear? Maybe there is a better way to connect the signature of the entity that create the repository with the value of the hashes?

aplanas · 2023-01-04T10:28:06Z

@Conan-Kudo ping. Can you remove the "decaying" tag?

kontura

Hello, sorry for the delay.

Unless I am wrong this breaks API only with the two functions I annotated, please can you rather create new ones? (the Python API should be fine as the new params are optional).

Otherwise in my opinion this is good to go and I would merge it.

src/parsehdr.c

src/parsepkg.h

src/xml_dump.h

Using the parameter '--filelists_ext', a new metadata file (filelists_ext) will be created. This new metadata is a copy of filelists, but will include a new attribute for the 'file' element, named 'hash', that contain the registered hash (SHA256 for example) for this file. Fix rpm-software-management#313 Signed-off-by: Alberto Planas <[email protected]>

aplanas · 2023-02-15T15:57:23Z

Another option instead to duplicate similar functions, can be to drop --filelist_ext and generate it always, and is the user who decides to ignore it or not

dralley · 2023-02-16T05:26:22Z

You probably don't need to "duplicate" per se, just have one implementation with a different signature which is called by the existing function, which would be turned into just a stub.

aplanas · 2023-02-16T07:45:28Z

just have one implementation with a different signature which is called by the existing function

Right, I used this approach.

aplanas force-pushed the fix_imadata branch from 31ced16 to e220cc3 Compare March 25, 2022 12:27

aplanas mentioned this pull request Mar 25, 2022

[backend] add "--filelists_ext" parameter for createrepo openSUSE/open-build-service#12358

Merged

aplanas force-pushed the fix_imadata branch from e220cc3 to 79515df Compare March 28, 2022 08:59

aplanas force-pushed the fix_imadata branch 5 times, most recently from 1f5504c to 11c274c Compare March 29, 2022 13:11

kontura reviewed Mar 29, 2022

View reviewed changes

src/xml_parser_filelists.c Outdated Show resolved Hide resolved

aplanas force-pushed the fix_imadata branch 3 times, most recently from b875d3f to 8d9db76 Compare March 29, 2022 14:34

aplanas force-pushed the fix_imadata branch 2 times, most recently from a59da74 to 34762c5 Compare March 30, 2022 13:41

aplanas marked this pull request as ready for review March 30, 2022 14:09

aplanas force-pushed the fix_imadata branch 4 times, most recently from 8506aad to e6bad40 Compare April 1, 2022 12:37

aplanas force-pushed the fix_imadata branch from e6bad40 to 2cc252b Compare April 1, 2022 15:33

dralley reviewed Apr 10, 2022

View reviewed changes

aplanas mentioned this pull request May 27, 2022

IMA policy enhancement proposal keylime/enhancements#71

Merged

kontura self-assigned this Jun 2, 2022

Conan-Kudo added needs discussion decaying labels Dec 7, 2022

aplanas force-pushed the fix_imadata branch from 2cc252b to 08586cb Compare December 8, 2022 08:25

aplanas force-pushed the fix_imadata branch 2 times, most recently from b3e6d79 to a347d6a Compare December 15, 2022 15:59

Conan-Kudo removed the decaying label Jan 4, 2023

kontura requested changes Feb 9, 2023

View reviewed changes

src/parsehdr.c Show resolved Hide resolved

src/parsepkg.h Outdated Show resolved Hide resolved

src/xml_dump.h Outdated Show resolved Hide resolved

aplanas force-pushed the fix_imadata branch from a347d6a to d4e7415 Compare February 15, 2023 15:51

kontura approved these changes Feb 16, 2023

View reviewed changes

kontura merged commit b254b44 into rpm-software-management:master Feb 16, 2023

kontura mentioned this pull request Feb 16, 2023

Rename filelists_ext to filelists-ext #348

Merged

aplanas mentioned this pull request Feb 16, 2023

Extract IMA hashes from local and remote repositories keylime/keylime#1305

Merged

aplanas deleted the fix_imadata branch February 22, 2023 10:28

kontura mentioned this pull request Apr 26, 2023

Fix change in python API of package files #362

Merged

Comments

Conversation

aplanas commented Mar 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aplanas commented Mar 24, 2022

Uh oh!

mlschroe commented Mar 25, 2022

Uh oh!

mlschroe commented Mar 25, 2022

Uh oh!

aplanas commented Mar 25, 2022

Uh oh!

aplanas commented Mar 25, 2022

Uh oh!

aplanas commented Mar 28, 2022

Uh oh!

kontura commented Mar 29, 2022

Uh oh!

Uh oh!

aplanas commented Mar 29, 2022

Uh oh!

aplanas commented Mar 29, 2022

Uh oh!

aplanas commented Mar 30, 2022

Uh oh!

aplanas commented Apr 1, 2022

Uh oh!

dralley commented Apr 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dralley Apr 10, 2022

Choose a reason for hiding this comment

Uh oh!

aplanas Apr 10, 2022

Choose a reason for hiding this comment

Uh oh!

dralley Apr 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aplanas Apr 10, 2022

Choose a reason for hiding this comment

Uh oh!

Conan-Kudo commented Dec 7, 2022

Uh oh!

aplanas commented Dec 8, 2022

Uh oh!

aplanas commented Jan 4, 2023

Uh oh!

kontura left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aplanas commented Feb 15, 2023

Uh oh!

dralley commented Feb 16, 2023

Uh oh!

aplanas commented Feb 16, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

aplanas commented Mar 24, 2022 •

edited

Loading

dralley commented Apr 10, 2022 •

edited

Loading

dralley Apr 10, 2022 •

edited

Loading