Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pkglistgen: solv_cache_update(): support update repo and 4 way merge (and merge skipping) #1406

Merged

Conversation

@jberry-suse
Copy link
Contributor

jberry-suse commented Feb 15, 2018

  • 21961da:
    pkglistgen: solv_cache_update(): support update repo and 4 way merge.

  • 460ab74:
    pkglistgen: do_dump_solv(): support update repos.

    Always pull down update repo and only overwrite if changed. Could rework
    more extensively to name based on hash of repomd.xml or somesuch, but would
    likely also want to remove old versions.

  • 84b142c:
    pkglistgen: solv_merge(): allow array of solv files to merge.

  • df4e23c:
    pkglistgen: solv_merge(): skip when inputs are older than merged.

  • a11192a:
    osclib/conf: leap: set download-baseurl-update.

Fixes #1396.

@jberry-suse jberry-suse force-pushed the jberry-suse:pkglistgen-dump-include-update branch from 6a11c3e to 538d56e Feb 15, 2018
@jberry-suse

This comment has been minimized.

Copy link
Contributor Author

jberry-suse commented Feb 15, 2018

Need to verify the end result is desired (perhaps via update tests?). It runs properly (completing a full run now for Leap). @lnussel is there a specific package I should see added?

@coveralls

This comment has been minimized.

Copy link

coveralls commented Feb 15, 2018

Coverage Status

Coverage decreased (-0.05%) to 30.701% when pulling c264ef1 on jberry-suse:pkglistgen-dump-include-update into 60cfab2 on openSUSE:master.

@jberry-suse jberry-suse force-pushed the jberry-suse:pkglistgen-dump-include-update branch from 538d56e to 21961da Feb 15, 2018
@jberry-suse

This comment has been minimized.

Copy link
Contributor Author

jberry-suse commented Feb 15, 2018

Inner diff, after refactoring for realizing mergesolv support more than 2 files so can avoid reduce() to turn three steps into one.

diff --git a/pkglistgen.py b/pkglistgen.py
index df8bd0d..123491e 100755
--- a/pkglistgen.py
+++ b/pkglistgen.py
@@ -782,25 +782,20 @@ class CommandLineInterface(ToolBase.CommandLineInterface):
 
                 solv_file_nonfree = os.path.join(
                     CACHEDIR, 'repo-{}-{}-{}.solv'.format(nonfree, repo, arch))
-                self.solv_merge(solv_file, solv_file_nonfree, solv_file_merged)
+                self.solv_merge(solv_file_merged, solv_file, solv_file_nonfree)
 
-    def solv_merge(self, solv1, solv2, solv_merged=None):
-        if not solv_merged:
-            # Determine name when called by reduce().
-            if '.merged.' not in solv1:
-                solv_merged = solv1.replace('.solv', '.merged.solv')
-            else:
-                solv_merged = solv1
+    def solv_merge(self, solv_merged, *solvs):
+        solvs = list(solvs) # From tuple.
 
         if os.path.exists(solv_merged):
-            modified = map(os.path.getmtime, [solv1, solv2, solv_merged])
-            if max(modified) <= modified[2]:
+            modified = map(os.path.getmtime, [solv_merged] + solvs)
+            if max(modified) <= modified[0]:
                 # The two inputs were modified before or at the same as merged.
                 logger.debug('merge skipped for {}'.format(solv_merged))
                 return solv_merged
 
         with open(solv_merged, 'w') as handle:
-            p = subprocess.Popen(['mergesolv', solv1, solv2], stdout=handle)
+            p = subprocess.Popen(['mergesolv'] + solvs, stdout=handle)
             p.communicate()
 
         if p.returncode:
@@ -930,7 +925,9 @@ class CommandLineInterface(ToolBase.CommandLineInterface):
 
         if name is not None:
             # Only update file if overwrite or different.
+            ofh.flush() # Ensure entirely written before comparing.
             if not opts.overwrite and os.path.exists(name) and filecmp.cmp(name + '.new', name, shallow=False):
+                logger.debug('file identical, skip dumping')
                 os.remove(name + '.new')
             else:
                 os.rename(name + '.new', name)
@@ -1297,10 +1294,10 @@ class CommandLineInterface(ToolBase.CommandLineInterface):
                 continue
 
             # Merge nonfree solv with free solv or copy free solv as merged.
+            merged = names[0].replace('.solv', '.merged.solv')
             if len(names) >= 2:
-                merged = reduce(self.solv_merge, names)
+                self.solv_merge(merged, *names)
             else:
-                merged = names[0].replace('.solv', '.merged.solv')
                 shutil.copyfile(names[0], merged)
             prior.add(merged)
 
@jberry-suse

This comment has been minimized.

Copy link
Contributor Author

jberry-suse commented Feb 15, 2018

Also needed a buffer flush to be able to compare files.

jberry-suse added 4 commits Feb 15, 2018
Always pull down update repo and only overwrite if changed. Could rework
more extensively to name based on hash of repomd.xml or somesuch, but would
likely also want to remove old versions.
@jberry-suse jberry-suse force-pushed the jberry-suse:pkglistgen-dump-include-update branch from 21961da to c264ef1 Feb 15, 2018
@lnussel

This comment has been minimized.

Copy link
Member

lnussel commented Feb 19, 2018

python-PyYAML for example was updated in 42.3 to the same version as we have in Leap 15

@jberry-suse jberry-suse merged commit f56ccb8 into openSUSE:master Feb 21, 2018
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@jberry-suse

This comment has been minimized.

Copy link
Contributor Author

jberry-suse commented Feb 21, 2018

I see the following addition in diff:

<obsoletepackage>python-PyYAML</obsoletepackage>
@jberry-suse jberry-suse deleted the jberry-suse:pkglistgen-dump-include-update branch Feb 21, 2018
@jberry-suse

This comment has been minimized.

Copy link
Contributor Author

jberry-suse commented Feb 21, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.