From 38821f47ba027709b22b7045158fec539a0e96b0 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 29 Mar 2019 12:38:37 -0400 Subject: [PATCH 01/10] [unpackfs] Make comment match code - The mismatch between "ir-chk" and the comment "to-check" led me to check (ha!) the output of rsync, and it outputs "to-chk" during small transfers; make sure the comment reflects what is actually being used to track progress (which is "ir-chk"). --- src/modules/unpackfs/main.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/modules/unpackfs/main.py b/src/modules/unpackfs/main.py index 436ec6a5c..7417793e8 100644 --- a/src/modules/unpackfs/main.py +++ b/src/modules/unpackfs/main.py @@ -110,17 +110,17 @@ def file_copy(source, dest, progress_cb): for line in iter(process.stdout.readline, b''): # small comment on this regexp. - # rsync outputs three parameters in the progress. - # xfer#x => i try to interpret it as 'file copy try no. x' - # to-check=x/y, where: - # - x = number of files yet to be checked - # - y = currently calculated total number of files. - # but if you're copying directory with some links in it, the xfer# + # rsync outputs three items in the progress (in parentheses) + # - xfer#x => Interpret it as 'file copy try no. x' + # - ir-chk=x/y, where: + # - x = number of files yet to be checked + # - y = currently calculated total number of files. + # - to-chk=x/y, which is similar but seems to only be emitted when + # progress is short; these are ignored. + # + # If you're copying directory with some links in it, the xfer# # might not be a reliable counter (for one increase of xfer, many # files may be created). - # In case of manjaro, we pre-compute the total number of files. - # therefore we can easily subtract x from y in order to get real files - # copied / processed count. m = re.findall(r'xfr#(\d+), ir-chk=(\d+)/(\d+)', line.decode()) if m: From dc2fafe3241ff181f70de6a2d73315be912ecf0e Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 29 Mar 2019 12:51:00 -0400 Subject: [PATCH 02/10] [unpackfs] to-chk is also progress information - ir-chk happens first, and then there's a phase with to-chk messages; use those as well. --- src/modules/unpackfs/main.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/modules/unpackfs/main.py b/src/modules/unpackfs/main.py index 7417793e8..4d077ce29 100644 --- a/src/modules/unpackfs/main.py +++ b/src/modules/unpackfs/main.py @@ -109,19 +109,20 @@ def file_copy(source, dest, progress_cb): ) for line in iter(process.stdout.readline, b''): - # small comment on this regexp. - # rsync outputs three items in the progress (in parentheses) + # rsync outputs progress in parentheses. Each line will have an + # xfer and a chk item (either ir-chk or to-chk) as follows: + # # - xfer#x => Interpret it as 'file copy try no. x' # - ir-chk=x/y, where: # - x = number of files yet to be checked # - y = currently calculated total number of files. - # - to-chk=x/y, which is similar but seems to only be emitted when - # progress is short; these are ignored. + # - to-chk=x/y, which is similar and happens once the ir-chk + # phase (collecting total files) is over. # # If you're copying directory with some links in it, the xfer# # might not be a reliable counter (for one increase of xfer, many # files may be created). - m = re.findall(r'xfr#(\d+), ir-chk=(\d+)/(\d+)', line.decode()) + m = re.findall(r'xfr#(\d+), ..-chk=(\d+)/(\d+)', line.decode()) if m: # we've got a percentage update From 9ce34782eea88dbec19e4aca82e4dc4de79cf79f Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 29 Mar 2019 16:09:11 -0400 Subject: [PATCH 03/10] [unpackfs] Avoid double / at end --- src/modules/unpackfs/main.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/modules/unpackfs/main.py b/src/modules/unpackfs/main.py index 4d077ce29..d406054d4 100644 --- a/src/modules/unpackfs/main.py +++ b/src/modules/unpackfs/main.py @@ -99,7 +99,8 @@ def file_copy(source, dest, progress_cb): # `source` *must* end with '/' otherwise a directory named after the source # will be created in `dest`: ie if `source` is "/foo/bar" and `dest` is # "/dest", then files will be copied in "/dest/bar". - source += "/" + if not source.endswith("/"): + source += "/" args = ['rsync', '-aHAXr'] args.extend(list_excludes(dest)) From 6d85fd3586dd6347aa45b068f93a0403fa1a3e3e Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 29 Mar 2019 16:24:22 -0400 Subject: [PATCH 04/10] [unpackfs] One last progress call afterwards --- src/modules/unpackfs/main.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/modules/unpackfs/main.py b/src/modules/unpackfs/main.py index d406054d4..c250931a0 100644 --- a/src/modules/unpackfs/main.py +++ b/src/modules/unpackfs/main.py @@ -102,6 +102,8 @@ def file_copy(source, dest, progress_cb): if not source.endswith("/"): source += "/" + num_files_copied = 0 # Gets updated through rsync output + args = ['rsync', '-aHAXr'] args.extend(list_excludes(dest)) args.extend(['--progress', source, dest]) @@ -137,6 +139,7 @@ def file_copy(source, dest, progress_cb): progress_cb(num_files_copied) process.wait() + progress_cb(num_files_copied) # Push towards 100% # 23 is the return code rsync returns if it cannot write extended # attributes (with -X) because the target file system does not support it, From fae0b8c2f83a460bb02fdac9e0171a40f737edbd Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 29 Mar 2019 16:48:46 -0400 Subject: [PATCH 05/10] [unpackfs] Re-jig progress reporting - rsync reports its own progress, and reports on files that find -type f doesn't. This meant that the numbers didn't match what was stored in entry.total - The ir-phase adds files to be handled; to-phase happens once ir-phase is over and the remaining files are processed. By adding the to-phase files, percentages over 100% were reported (in part because the number of files doesn't match). - Update expected entries total from rsync output. - Re-jig computation of how done everything is: tally it up in integers, and do only one global progress percentage. --- src/modules/unpackfs/main.py | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/modules/unpackfs/main.py b/src/modules/unpackfs/main.py index c250931a0..40cd8c628 100644 --- a/src/modules/unpackfs/main.py +++ b/src/modules/unpackfs/main.py @@ -136,10 +136,10 @@ def file_copy(source, dest, progress_cb): # I guess we're updating every 100 files... if num_files_copied % 100 == 0: - progress_cb(num_files_copied) + progress_cb(num_files_copied, num_files_total_local) process.wait() - progress_cb(num_files_copied) # Push towards 100% + progress_cb(num_files_copied, num_files_total_local) # Push towards 100% # 23 is the return code rsync returns if it cannot write extended # attributes (with -X) because the target file system does not support it, @@ -177,14 +177,19 @@ class UnpackOperation: """ progress = float(0) + done = 0 + total = 0 + complete = 0 for entry in self.entries: if entry.total == 0: continue + total += entry.total + done += entry.copied + if entry.total == entry.copied: + complete += 1 - partialprogress = 0.05 # Having a total !=0 gives 5% - - partialprogress += 0.95 * (entry.copied / float(entry.total)) - progress += partialprogress / len(self.entries) + if done > 0 and total > 0: + progress = 0.05 + (0.90 * done / total) + (0.05 * complete / len(self.entries)) job.setprogress(progress) @@ -263,12 +268,13 @@ class UnpackOperation: :param imgmountdir: :return: """ - def progress_cb(copied): + def progress_cb(copied, total): """ Copies file to given destination target. :param copied: """ entry.copied = copied + entry.total = total self.report_progress() try: From 2a6bf50621416ca519e7b0f143ff58e51c3f9f88 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 29 Mar 2019 16:53:02 -0400 Subject: [PATCH 06/10] [unpackfs] Don't let ir-phase reduce total number of files --- src/modules/unpackfs/main.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/modules/unpackfs/main.py b/src/modules/unpackfs/main.py index 40cd8c628..de9108254 100644 --- a/src/modules/unpackfs/main.py +++ b/src/modules/unpackfs/main.py @@ -274,7 +274,8 @@ class UnpackOperation: :param copied: """ entry.copied = copied - entry.total = total + if total > entry.total: + entry.total = total self.report_progress() try: From d87badbf45ab77819172052079a6b40edcc522b3 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 29 Mar 2019 17:22:47 -0400 Subject: [PATCH 07/10] [unpackfs] Add test with too-small destination FS --- src/modules/unpackfs/runtests.sh | 12 ++++++++++++ src/modules/unpackfs/tests/9.global | 3 +++ src/modules/unpackfs/tests/9.job | 6 ++++++ 3 files changed, 21 insertions(+) create mode 100644 src/modules/unpackfs/tests/9.global create mode 100644 src/modules/unpackfs/tests/9.job diff --git a/src/modules/unpackfs/runtests.sh b/src/modules/unpackfs/runtests.sh index 2b9b704c0..be175e0cd 100644 --- a/src/modules/unpackfs/runtests.sh +++ b/src/modules/unpackfs/runtests.sh @@ -7,9 +7,21 @@ mkdir /tmp/unpackfs-test-run-rootdir3 # For test 7 mkdir /tmp/unpackfs-test-run-rootdir3/realdest +# For test 9 +mkdir /tmp/unpackfs-test-run-rootdir3/smalldest +if test 0 = $( id -u ) ; then + mount -t tmpfs -o size=32M tmpfs /tmp/unpackfs-test-run-rootdir3/smalldest + dd if=/dev/zero of=/tmp/unpackfs-test-run-rootdir3/smalldest/bogus.zero bs=1M count=1 +fi + # Run tests sh "$SRCDIR/../testpythonrun.sh" unpackfs +# Cleanup test 9 +if test 0 = $( id -u ) ; then + umount /tmp/unpackfs-test-run-rootdir3/smalldest +fi + # Cleanup test 7 rm -rf /tmp/unpackfs-test-run-rootdir3/realdest diff --git a/src/modules/unpackfs/tests/9.global b/src/modules/unpackfs/tests/9.global new file mode 100644 index 000000000..82ca8f6f6 --- /dev/null +++ b/src/modules/unpackfs/tests/9.global @@ -0,0 +1,3 @@ +# This test uses a small destination FS, to make rsync fail +--- +rootMountPoint: /tmp/unpackfs-test-run-rootdir3/ diff --git a/src/modules/unpackfs/tests/9.job b/src/modules/unpackfs/tests/9.job new file mode 100644 index 000000000..7eabd497c --- /dev/null +++ b/src/modules/unpackfs/tests/9.job @@ -0,0 +1,6 @@ +# This test uses a small destination FS, to make rsync fail +--- +unpack: + - source: . + sourcefs: ext4 + destination: smalldest From 5c4f2423f8a14befa5ad6bf58dd82b055e82235e Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 29 Mar 2019 17:27:26 -0400 Subject: [PATCH 08/10] [unpackfs] Fix error in error-handling (warn -> warning) --- src/modules/unpackfs/main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/unpackfs/main.py b/src/modules/unpackfs/main.py index de9108254..5bacb373d 100644 --- a/src/modules/unpackfs/main.py +++ b/src/modules/unpackfs/main.py @@ -154,7 +154,7 @@ def file_copy(source, dest, progress_cb): # https://bugzilla.redhat.com/show_bug.cgi?id=868755#c50 # for the same issue in Anaconda, which uses a similar workaround. if process.returncode != 0 and process.returncode != 23: - utils.warn("rsync failed with error code {}.".format(process.returncode)) + utils.warning("rsync failed with error code {}.".format(process.returncode)) return _("rsync failed with error code {}.").format(process.returncode) return None From fb412c177c661f0b573f6b3486e54ed4957db674 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 29 Mar 2019 17:32:52 -0400 Subject: [PATCH 09/10] [unpackfs] Improve human-readable name --- src/modules/unpackfs/main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/unpackfs/main.py b/src/modules/unpackfs/main.py index 5bacb373d..dd9439a2c 100644 --- a/src/modules/unpackfs/main.py +++ b/src/modules/unpackfs/main.py @@ -37,7 +37,7 @@ _ = gettext.translation("calamares-python", fallback=True).gettext def pretty_name(): - return _("Installing filesystems.") + return _("Filling up filesystems.") class UnpackEntry: From e2d3f2d88592d3143c988c9fed0697ac5b170277 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 29 Mar 2019 17:34:39 -0400 Subject: [PATCH 10/10] Changes: say something about improved UnpackFS - Although nothing specific was done, I'm fairly sure that the issue isn't worth keeping around. FIXES #565 --- CHANGES | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGES b/CHANGES index a3d9ae9f7..9fa7c2d6b 100644 --- a/CHANGES +++ b/CHANGES @@ -39,6 +39,7 @@ This release contains contributions from (alphabetically by first name): usually only visible when the module runs as part of the *exec* step, when the module's *pretty name* is displayed. In addition, some error messages are now translated. + * *UnpackFS* module: improved progress reporting and tests. # 3.2.4 (2019-02-12) #