Add detection of %posttrans and %pretrans scriptlet from rpm header

[wswsmao]

In specs files, user may use %posttrans and %pretrans scriptlet like this:
Requires(pretrans): test_pkg
Requires(posttrans): test_pkg
This commit add detection to the above special scriptlet

[ppisar]

Could you rewrite the commit message? "add posttrans and pretrans for parsehdr" is not good because that's already visible in the code. Instead use the text you wrote into the pull request text. The commit message should document a reason why it commit exists.

[wswsmao]

Could you rewrite the commit message? "add posttrans and pretrans for parsehdr" is not good because that's already visible in the code. Instead use the text you wrote into the pull request text. The commit message should document a reason why it commit exists.

@ppisar done

[ppisar]

Thanks for the commit message.

Handling RPMSENSE_PRETRANS as RPMSENSE_PREREQ is fine.

But I'm not sure whether RPMSENSE_POSTTRANS should be exported into XML with "pre" attribute. RPM documents the distinction:

* `pretrans`

  Denotes the dependency must be present before the transaction starts,
  and cannot be satisified by added packages in a transaction. As such,
  it does not affect transaction ordering. A pretrans-dependency is
  free to be removed after the install-transaction completes.

  Also relates to `%pretrans` and `%preuntrans` scriptlet execution.

* `posttrans` 

  Denotes the dependency must be present at the end of transaction, ie
  cannot be removed during the transaction. As such, it does not affect
  transaction ordering. A posttrans-dependency is free to be removed
  after the the install-transaction completes.

  Also relates to `%posttrans` and `%postuntrans` scriptlet execution.

Notice that posttrans can be satisfied by installing a package during that transaction. Exporting the dependency with "pre" attribute is stronger.

I don't say RPMSENSE_POSTTRANS should not be recorded in the XML output. But marking the with "pre" attribute does not seem right to me. I can see that RPMSENSE_SCRIPT_POST is marked with "pre" attribute. I feel that it's actually a bug. We need it find someone more knowledgeable in this area.

[ppisar]

@j-mracek, have you an idea what "<rpm:entry pre=1/>" attribute in the primary.xml means? Is using that attribute for RPMSENSE_POSTTRANS dependencies correct? I cannot find anything about post dependencies in libsolv.

[ppisar]

By the way. @wswsmao, this feature should have a test. There is test_xmlfile_add_pkg() in tests/python/tests/test_xml_file.py and tests/testdata/specs/fake-Archer.spec which could be enhanced. Look for "foof" dependency.

[ppisar]

But I'm not sure whether RPMSENSE_POSTTRANS should be exported into XML with "pre" attribute. RPM documents the distinction:
[...]
I don't say RPMSENSE_POSTTRANS should not be recorded in the XML output. But marking the with "pre" attribute does not seem right to me. I can see that RPMSENSE_SCRIPT_POST is marked with "pre" attribute. I feel that it's actually a bug. We need it find someone more knowledgeable in this area.

I was thinking about it and maybe the semantics of "pre" attribute is different for the package mangers. Maybe libsolv does not care which package is installed first and which later. Maybe libsolv only cares about a state before and after the transaction, it does not hassle with the order. Maybe it uses "pre" attribute to mark packages which must be present during the installation, but can be uninstalled after the transaction. (Computing the exact order is then left to RPM.)

If this theory is correct, then it you patch is also correct.

[wswsmao]

By the way. @wswsmao, this feature should have a test. There is test_xmlfile_add_pkg() in tests/python/tests/test_xml_file.py and tests/testdata/specs/fake-Archer.spec which could be enhanced. Look for "foof" dependency.

Hi @ppisar , I have enhanced the test，but I get this error:

2: ======================================================================
2: FAIL: test_package_archer (tests.test_package.TestCasePackage.test_package_archer)
2: ----------------------------------------------------------------------
2: Traceback (most recent call last):
2:   File "/root/build/BUILD/createrepo_c-0.20.1/tests/python/tests/test_package.py", line 80, in test_package_archer
2:     self.assertEqual(pkg.requires, [
2: AssertionError: Lists differ: [('fo[225 chars]True)] != [('fo[225 chars]True), ('foog', 'EQ', '0', '7', None, True), ([31 chars]rue)]
2: 
2: Second list contains 2 additional elements.
2: First extra element 6:
2: ('foog', 'EQ', '0', '7', None, True)
2: 
2:   [('fooa', 'LE', '0', '2', None, False),
2:    ('foob', 'GE', '0', '1.0.0', '1', False),
2:    ('fooc', 'EQ', '0', '3', None, False),
2:    ('food', 'LT', '0', '4', None, False),
2:    ('fooe', 'GT', '0', '5', None, False),
2: -  ('foof', 'EQ', '0', '6', None, True)]
2: ?                                      ^
2: 
2: +  ('foof', 'EQ', '0', '6', None, True),
2: ?                                      ^
2: 
2: +  ('foog', 'EQ', '0', '7', None, True),
2: +  ('fooh', 'EQ', '0', '8', None, True)]
2: 
2: ======================================================================
2: FAIL: test_sqlite_primary (tests.test_sqlite.TestCaseSqlite.test_sqlite_primary)
2: ----------------------------------------------------------------------
2: Traceback (most recent call last):
2:   File "/root/build/BUILD/createrepo_c-0.20.1/tests/python/tests/test_sqlite.py", line 192, in test_sqlite_primary
2:     self.assertEqual(con.execute("select * from requires").fetchall(),
2: AssertionError: Lists differ: [('fo[255 chars]RUE')] != [('fo[255 chars]RUE'), ('foog', 'EQ', '0', '7', None, 1, 'TRUE[41 chars]UE')]
2: 
2: Second list contains 2 additional elements.
2: First extra element 6:
2: ('foog', 'EQ', '0', '7', None, 1, 'TRUE')
2: 
2:   [('fooa', 'LE', '0', '2', None, 1, 'FALSE'),
2:    ('foob', 'GE', '0', '1.0.0', '1', 1, 'FALSE'),
2:    ('fooc', 'EQ', '0', '3', None, 1, 'FALSE'),
2:    ('food', 'LT', '0', '4', None, 1, 'FALSE'),
2:    ('fooe', 'GT', '0', '5', None, 1, 'FALSE'),
2: -  ('foof', 'EQ', '0', '6', None, 1, 'TRUE')]
2: ?                                           ^
2: 
2: +  ('foof', 'EQ', '0', '6', None, 1, 'TRUE'),
2: ?                                           ^
2: 
2: +  ('foog', 'EQ', '0', '7', None, 1, 'TRUE'),
2: +  ('fooh', 'EQ', '0', '8', None, 1, 'TRUE')]
2: 
2: ======================================================================
2: FAIL: test_xmlfile_add_pkg (tests.test_xml_file.TestCaseXmlFile.test_xmlfile_add_pkg)
2: ----------------------------------------------------------------------
2: Traceback (most recent call last):
2:   File "/root/build/BUILD/createrepo_c-0.20.1/tests/python/tests/test_xml_file.py", line 192, in test_xmlfile_add_pkg
2:     self.assertEqual(primary.read(),
2: AssertionError: '<?xm[1891 chars]n    </rpm:requires>\n    <rpm:conflicts>\n   [789 chars]ata>' != '<?xm[1891 chars]n      <rpm:entry name="foog" flags="EQ" epoch[927 chars]ata>'
2: Diff is 2932 characters long. Set self.maxDiff to None to see it.
2: 
2: ----------------------------------------------------------------------
2: Ran 121 tests in 0.188s
2: 
2: FAILED (failures=3)
2/2 Test #2: test_python ......................***Failed    0.29 sec

Is there any other file need be updated ?

[ppisar]

First of all, please rebase your branch to current master. Your branch is missing 25 recent commit and therefore it does not build for me.

[wswsmao]

First of all, please rebase your branch to current master. Your branch is missing 25 recent commit and therefore it does not build for me.

@ppisar rebase done

[ppisar]

It reason is that the tests use pregenerated RPM packages from tests/testdata/packages directory without rebuilding them from spec files located in tests/testdata/specs. That means you need to manually rebuild tests/testdata/packages/Archer-3.4.5-6.x86_64.rpm from tests/testdata/specs/fake-Archer.spec and add that binary package into the commit. When you build the package, make sure it keeps using Xz compression (a synthetic run-time dependency on "rpmlib(PayloadIsXz)"). The compression is controlled with _binary_payload macro.

That helps you to proceed with the tests. You will get two new failures on an unexpected size of primary.xml and a package ID of Archer-3.4.5-6.x86_64.rpm. After correcting them, the tests could pass.

[wswsmao]

It reason is that the tests use pregenerated RPM packages from tests/testdata/packages directory without rebuilding them from spec files located in tests/testdata/specs. That means you need to manually rebuild tests/testdata/packages/Archer-3.4.5-6.x86_64.rpm from tests/testdata/specs/fake-Archer.spec and add that binary package into the commit. When you build the package, make sure it keeps using Xz compression (a synthetic run-time dependency on "rpmlib(PayloadIsXz)"). The compression is controlled with _binary_payload macro.

That helps you to proceed with the tests. You will get two new failures on an unexpected size of primary.xml and a package ID of Archer-3.4.5-6.x86_64.rpm. After correcting them, the tests could pass.

ok，i will try it

[wswsmao]

It reason is that the tests use pregenerated RPM packages from tests/testdata/packages directory without rebuilding them from spec files located in tests/testdata/specs. That means you need to manually rebuild tests/testdata/packages/Archer-3.4.5-6.x86_64.rpm from tests/testdata/specs/fake-Archer.spec and add that binary package into the commit. When you build the package, make sure it keeps using Xz compression (a synthetic run-time dependency on "rpmlib(PayloadIsXz)"). The compression is controlled with _binary_payload macro.

That helps you to proceed with the tests. You will get two new failures on an unexpected size of primary.xml and a package ID of Archer-3.4.5-6.x86_64.rpm. After correcting them, the tests could pass.

Hi @ppisar , I think I have adapted enough files, but there is still error like this:

2: 
2: 
2: (process:1975761): C_CREATEREPOLIB-WARNING **: 14:30:30.268: read_header: rpmReadPackageFile() error
2: 
2: (process:1975761): C_CREATEREPOLIB-WARNING **: 14:30:30.268: read_header: rpmReadPackageFile() error
2: ..................................F..........................................................
2: ======================================================================
2: FAIL: test_contentstat (tests.test_contentstat.TestCaseContentStat.test_contentstat)
2: ----------------------------------------------------------------------
2: Traceback (most recent call last):
2:   File "/root/build/BUILD/createrepo_c-0.20.1/tests/python/tests/test_contentstat.py", line 43, in test_contentstat
2:     self.assertEqual(cs.checksum, "67bc6282915fad80dc11f3d7c3210977a0bde"\
2: AssertionError: 'd447983b39363a6519067ce477a7fc64409b4900e0160da68d66b25207a0408d' != '67bc6282915fad80dc11f3d7c3210977a0bde05a762256d86083c2447d425776'
2: - d447983b39363a6519067ce477a7fc64409b4900e0160da68d66b25207a0408d
2: + 67bc6282915fad80dc11f3d7c3210977a0bde05a762256d86083c2447d425776
2: 

what's more, is there any tools or script coud simply this work?

[ppisar]

That piece of test checks for an SHA-256 checksum of an uncompressed content of the primary.xml file. A name of the file is stored in "path" variable.

I don't think there is a script for updating the tests. Just use the "d447983b39363a6519067ce477a7fc64409b4900e0160da68d66b25207a0408d" value. You can get it when you spawn an interactive shell from the test and run something like "zcat /tmp/createrepo_ctest-r5x4jlba/primary.xml.gz | sha256sum -b".

(I think that some tests like the checksum are pointless and that the rest should be automated, so the packages are built from spec files automatically and package checksums retrieved automatically. However, I'm not an author, nor long-term maintainer of createrepo_c.)

[wswsmao]

tests/python/tests/test_contentstat.p

@ppisar done

[ppisar]

Thanks for the tests. They look good. I believe the single failure in testing-farm:fedora-40-x86_64:createrepo_c-tests is unrelated. I only rerun that test to see whether the failure is random or deterministic.

I talked to other DNF developers and they think that handling post-dependencies with pre="1" attribute is fine because libsolv indeed uses it as a dependency on a package which must be present, but can be removed later.