Reliably use modified repodata for get_new_repoclosure
I noticed that https://openqa.fedoraproject.org/tests/4471016 passed when it should not have: it should have caught that the new gstreamer1-plugins-good package had a dependency issue. I was able to reproduce this locally. Some debugging indicated that, in get_new_repoclosure, the base repository that should have been modified was not behaving as if it was modified. I think we've been relying on DNF's cache, here. We only actually used the modified repodata directly in get_modified_repoclosure. In get_new_repoclosure we were configuring the repo with the same name, but with its original unmodified path. I think we expected that DNF would cache the modified metadata and re-use it. This obviously worked at some point (because we *have* had correct failures in get_new_repoclosure), but it's obviously fragile. Let's improve it by creating the temporary directory where we store the modified repodata in main() and passing it to both get_modified_repoclosure() and get_new_repoclosure(); this way get_new_repoclosure() can access and use the actual modified repodata. In my local testing, this fixes the bug and we now correctly find the dependency error. Signed-off-by: Adam Williamson <awilliam@redhat.com>
This commit is contained in:
parent
4b639cb97e
commit
c2b6ce2a72
2 changed files with 77 additions and 66 deletions
123
rmdepcheck.py
123
rmdepcheck.py
|
|
@ -193,7 +193,11 @@ def get_base_repoclosure(baserepos: Iterable[str], nmbaserepos: Iterable[str]) -
|
|||
|
||||
# pylint: disable-next=too-many-locals
|
||||
def get_modified_repoclosure(
|
||||
mrepos: Iterable[str], nmrepos: Iterable[str], nrepos: Iterable[str], removes: Iterable[str]
|
||||
mrepos: Iterable[str],
|
||||
mreposdir: str,
|
||||
nmrepos: Iterable[str],
|
||||
nrepos: Iterable[str],
|
||||
removes: Iterable[str],
|
||||
) -> str:
|
||||
"""Does the repository metadata modification (the clever bit!) and
|
||||
returns the modified repoclosure text. Non-modified base repos,
|
||||
|
|
@ -201,56 +205,54 @@ def get_modified_repoclosure(
|
|||
available to the solver; only modified base repos are checked.
|
||||
"""
|
||||
args = DNFARGS + ["repoclosure"]
|
||||
# place to stash the modified repos
|
||||
with tempfile.TemporaryDirectory() as mreposdir:
|
||||
for mrepo in mrepos:
|
||||
mrepodir = f"{mreposdir}/{hash_repo(mrepo)}"
|
||||
os.makedirs(f"{mrepodir}/repodata")
|
||||
repomdfn = f"{mrepodir}/repodata/repomd.xml"
|
||||
get_file(f"{mrepo}/repodata/repomd.xml", repomdfn)
|
||||
et.register_namespace("", "http://linux.duke.edu/metadata/repo")
|
||||
repomdtree = et.parse(repomdfn)
|
||||
repomdroot = repomdtree.getroot()
|
||||
primary = get_primary(repomdroot)
|
||||
primfn = download_primary(primary, mrepo, mrepodir)
|
||||
csum, size, opensum, opensize = replace_primary(primfn, removes)
|
||||
for mrepo in mrepos:
|
||||
mrepodir = f"{mreposdir}/{hash_repo(mrepo)}"
|
||||
os.makedirs(f"{mrepodir}/repodata")
|
||||
repomdfn = f"{mrepodir}/repodata/repomd.xml"
|
||||
get_file(f"{mrepo}/repodata/repomd.xml", repomdfn)
|
||||
et.register_namespace("", "http://linux.duke.edu/metadata/repo")
|
||||
repomdtree = et.parse(repomdfn)
|
||||
repomdroot = repomdtree.getroot()
|
||||
primary = get_primary(repomdroot)
|
||||
primfn = download_primary(primary, mrepo, mrepodir)
|
||||
csum, size, opensum, opensize = replace_primary(primfn, removes)
|
||||
|
||||
# modify the repomd
|
||||
mfind(primary, "repo:checksum", XMLNS).text = csum
|
||||
mfind(primary, "repo:size", XMLNS).text = str(size)
|
||||
mfind(primary, "repo:open-checksum", XMLNS).text = opensum
|
||||
mfind(primary, "repo:open-size", XMLNS).text = str(opensize)
|
||||
mfind(primary, "repo:location", XMLNS).attrib[
|
||||
"href"
|
||||
] = f"repodata/{csum}-primary.xml.zst"
|
||||
# requires Python 3.10:
|
||||
# notprimary = repomdroot.findall("repo:data[@type]", XMLNS)
|
||||
alldata = repomdroot.findall("repo:data[@type]", XMLNS)
|
||||
notprimary = [data for data in alldata if data is not primary]
|
||||
for item in notprimary:
|
||||
repomdroot.remove(item)
|
||||
et.register_namespace("", "http://linux.duke.edu/metadata/repo")
|
||||
repomdtree.write(repomdfn)
|
||||
# add the modified repo to the repoclosure command
|
||||
args.extend(["--repofrompath", f"{hash_repo(mrepo)},{mrepodir}"])
|
||||
# modify the repomd
|
||||
mfind(primary, "repo:checksum", XMLNS).text = csum
|
||||
mfind(primary, "repo:size", XMLNS).text = str(size)
|
||||
mfind(primary, "repo:open-checksum", XMLNS).text = opensum
|
||||
mfind(primary, "repo:open-size", XMLNS).text = str(opensize)
|
||||
mfind(primary, "repo:location", XMLNS).attrib["href"] = f"repodata/{csum}-primary.xml.zst"
|
||||
# requires Python 3.10:
|
||||
# notprimary = repomdroot.findall("repo:data[@type]", XMLNS)
|
||||
alldata = repomdroot.findall("repo:data[@type]", XMLNS)
|
||||
notprimary = [data for data in alldata if data is not primary]
|
||||
for item in notprimary:
|
||||
repomdroot.remove(item)
|
||||
et.register_namespace("", "http://linux.duke.edu/metadata/repo")
|
||||
repomdtree.write(repomdfn)
|
||||
# add the modified repo to the repoclosure command
|
||||
args.extend(["--repofrompath", f"{hash_repo(mrepo)},{mrepodir}"])
|
||||
|
||||
# now add the non-modified base repos
|
||||
for nmrepo in nmrepos:
|
||||
args.extend(["--repofrompath", f"{hash_repo(nmrepo)},{nmrepo}"])
|
||||
# now add the non-modified base repos
|
||||
for nmrepo in nmrepos:
|
||||
args.extend(["--repofrompath", f"{hash_repo(nmrepo)},{nmrepo}"])
|
||||
|
||||
# now add the new package repos
|
||||
for nrepo in nrepos:
|
||||
args.extend(["--repofrompath", f"{hash_repo(nrepo)},{nrepo}"])
|
||||
# now add the new package repos
|
||||
for nrepo in nrepos:
|
||||
args.extend(["--repofrompath", f"{hash_repo(nrepo)},{nrepo}"])
|
||||
|
||||
# finally, add the check arg
|
||||
args.append("--check")
|
||||
args.append(",".join([hash_repo(mrepo) for mrepo in mrepos]))
|
||||
# finally, add the check arg
|
||||
args.append("--check")
|
||||
args.append(",".join([hash_repo(mrepo) for mrepo in mrepos]))
|
||||
|
||||
ret = SUBPCAPTURE(args).stdout
|
||||
ret = SUBPCAPTURE(args).stdout
|
||||
return ret
|
||||
|
||||
|
||||
def get_new_repoclosure(baserepos: Iterable[str], nrepo: str) -> str:
|
||||
def get_new_repoclosure(
|
||||
mrepos: Iterable[str], mreposdir: str, nmrepos: Iterable[str], nrepo: str
|
||||
) -> str:
|
||||
"""Gets and returns repoclosure text for the new repository; this
|
||||
is effectively an installability check. All base repos are
|
||||
available to the solver but are not checked. Note this is run
|
||||
|
|
@ -258,7 +260,10 @@ def get_new_repoclosure(baserepos: Iterable[str], nrepo: str) -> str:
|
|||
versions of the modifiable base repositories.
|
||||
"""
|
||||
cmdargs = DNFARGS + ["repoclosure"]
|
||||
for repo in baserepos:
|
||||
for mrepo in mrepos:
|
||||
mrepodir = f"{mreposdir}/{hash_repo(mrepo)}"
|
||||
cmdargs.extend(["--repofrompath", f"{hash_repo(mrepo)},{mrepodir}"])
|
||||
for repo in nmrepos:
|
||||
cmdargs.extend(["--repofrompath", f"{hash_repo(repo)},{repo}"])
|
||||
cmdargs.extend(["--repofrompath", f"{hash_repo(nrepo)},{nrepo}", "--check", hash_repo(nrepo)])
|
||||
return SUBPCAPTURE(cmdargs).stdout
|
||||
|
|
@ -412,21 +417,25 @@ def main() -> None:
|
|||
|
||||
baserc = parse_repoclosure(get_base_repoclosure(args.baserepos, args.nmbaserepos))
|
||||
|
||||
# get the modified rpmclosure output
|
||||
modrc = parse_repoclosure(
|
||||
get_modified_repoclosure(args.baserepos, args.nmbaserepos, nrepos, sources)
|
||||
)
|
||||
|
||||
# figure out the diffs
|
||||
newerrors = [dep for dep in modrc if dep not in baserc]
|
||||
fixederrors = [dep for dep in baserc if dep not in modrc]
|
||||
newrc = []
|
||||
if args.repo:
|
||||
# get repoclosure on new repo - this is an installability test
|
||||
newrc = parse_repoclosure(
|
||||
get_new_repoclosure(args.baserepos + args.nmbaserepos, args.repo)
|
||||
# place to stash the modified repos
|
||||
with tempfile.TemporaryDirectory() as mreposdir:
|
||||
# get the modified rpmclosure output
|
||||
modrc = parse_repoclosure(
|
||||
get_modified_repoclosure(
|
||||
args.baserepos, mreposdir, args.nmbaserepos, nrepos, sources
|
||||
)
|
||||
)
|
||||
|
||||
# figure out the diffs
|
||||
newerrors = [dep for dep in modrc if dep not in baserc]
|
||||
fixederrors = [dep for dep in baserc if dep not in modrc]
|
||||
newrc = []
|
||||
if args.repo:
|
||||
# get repoclosure on new repo - this is an installability test
|
||||
newrc = parse_repoclosure(
|
||||
get_new_repoclosure(args.baserepos, mreposdir, args.nmbaserepos, args.repo)
|
||||
)
|
||||
|
||||
# output
|
||||
if args.json:
|
||||
jsonout = {}
|
||||
|
|
|
|||
|
|
@ -150,9 +150,10 @@ def test_get_modified_repoclosure():
|
|||
with open(f"{TESTDATA}/test_get_modified_repoclosure.txt", "r", encoding="utf-8") as testfh:
|
||||
expected = testfh.read()
|
||||
expected = expected.replace("{HASH}", rmdepcheck.hash_repo(brepo))
|
||||
ret = rmdepcheck.get_modified_repoclosure(
|
||||
[brepo], [], [f"file://{REPOS}/new"], ["aaa", "ccc", "eee", "fff", "ggg"]
|
||||
)
|
||||
with tempfile.TemporaryDirectory() as tempdir:
|
||||
ret = rmdepcheck.get_modified_repoclosure(
|
||||
[brepo], tempdir, [], [f"file://{REPOS}/new"], ["aaa", "ccc", "eee", "fff", "ggg"]
|
||||
)
|
||||
assert ret == expected
|
||||
|
||||
|
||||
|
|
@ -161,7 +162,8 @@ def test_get_new_repoclosure():
|
|||
with open(f"{TESTDATA}/test_get_new_repoclosure.txt", "r", encoding="utf-8") as testfh:
|
||||
expected = testfh.read()
|
||||
expected = expected.replace("{HASH}", rmdepcheck.hash_repo(nrepo))
|
||||
ret = rmdepcheck.get_new_repoclosure([f"file://{REPOS}/base"], nrepo)
|
||||
with tempfile.TemporaryDirectory() as tempdir:
|
||||
ret = rmdepcheck.get_new_repoclosure([f"file://{REPOS}/base"], tempdir, [], nrepo)
|
||||
assert ret == expected
|
||||
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue