Ticket #3781 (closed defect: fixed)

Opened 3 years ago

Last modified 3 years ago

extfs: Some tests fail on Gentoo. Reason(s) yet unknown.

Reported by: slyfox Owned by: mooffie
Priority: major Milestone: 4.8.20
Component: tests Version: 4.8.19
Keywords: Cc:
Blocked By: #3786, #3787 Blocking:
Branch state: no branch Votes for changeset:

Description

~/dev/git/mc $ src/mc --version
GNU Midnight Commander 4.8.19
Built with GLib 2.50.3
Using the S-Lang library with terminfo database
With builtin Editor
With subshell support as default
With support for background operations
With mouse support on xterm and Linux console
With support for X11 events
With internationalization support
With multiple codepages support
Virtual File Systems: cpiofs, tarfs, sfs, extfs, ftpfs, sftpfs, fish
Data types: char: 8; int: 32; long: 64; void *: 64; size_t: 64; off_t: 64;

Fails make check as:

============================================================================
Testsuite summary for /src/vfs/extfs/helpers-list
============================================================================
# TOTAL: 1
# PASS:  0
# SKIP:  0
# XFAIL: 0
# FAIL:  1
# XPASS: 0
# ERROR: 0
============================================================================
See tests/src/vfs/extfs/helpers-list/test-suite.log
============================================================================

Attachments

test-suite.log (6.4 KB) - added by slyfox 3 years ago.
test-suite.log
en_US-test-suite.log (4.6 KB) - added by slyfox 3 years ago.
updated test log

Change History

Changed 3 years ago by slyfox

test-suite.log

comment:1 Changed 3 years ago by slyfox

The first error here is:

Testing /home/slyfox/dev/git/mc/tests/src/vfs/extfs/helpers-list/data/rpm.custom.input
/home/slyfox/dev/git/mc/tests/src/vfs/extfs/helpers-list/data/rpm.custom.output /tmp/rpm.actual-parsed-output.5KFukg2S differ: char 42, line 1

ERROR: rpm has produced output that's different than the expected output.

  Expected output (after parsing): /home/slyfox/dev/git/mc/tests/src/vfs/extfs/helpers-list/data/rpm.custom.output
  Actual output (after parsing): /tmp/rpm.actual-parsed-output.5KFukg2S

This might mean that a bug was introduced into rpm. Or that a bug was fixed.
Please compare the files.

If the actual output is the correct one, just copy the latter file
onto the former (and commit to the git repository).

Tip: invoke this program with '--mcdiff' to automatically launch
mcdiff to visually inspect the diff.

It's a surprisingly uninformative message. To get actual failure I need to diff a thing:

$ diff -U1 /home/slyfox/dev/git/mc/tests/src/vfs/extfs/helpers-list/data/rpm.custom.output/tmp/rpm.actual-parsed-output.5KFukg2S

--- /home/slyfox/dev/git/mc/tests/src/vfs/extfs/helpers-list/data/rpm.custom.output     2017-01-22 16:42:40.730438199 +0000
+++ /tmp/rpm.actual-parsed-output.5KFukg2S      2017-03-06 09:04:45.828152887 +0000
@@ -1,2 +1,2 @@
--r--r--r--   1        0        0        539 2017-01-05 00:00:00 HEADER
+-r--r--r--   1        0        0        597 2017-01-05 00:00:00 HEADER
 -r-xr-xr-x   1        0        0         39 2017-01-05 00:00:00 INSTALL
@@ -15,2 +15,3 @@
 -r--r--r--   1        0        0          0 2017-01-05 00:00:00 INFO/DESCRIPTION
+-r--r--r--   1        0        0          0 2017-01-05 00:00:00 INFO/SUMMARY
 -r--r--r--   1        0        0          0 2017-01-05 00:00:00 INFO/SCRIPTS/PRETRANS
@@ -30,2 +31,9 @@
 -r--r--r--   1        0        0          0 2017-01-05 00:00:00 INFO/SCRIPTS/VERIFYSCRIPTPROG
+-r--r--r--   1        0        0          0 2017-01-05 00:00:00 INFO/PACKAGER
+-r--r--r--   1        0        0          0 2017-01-05 00:00:00 INFO/URL
+-r--r--r--   1        0        0          0 2017-01-05 00:00:00 INFO/EPOCH
+-r--r--r--   1        0        0          0 2017-01-05 00:00:00 INFO/LICENSE
+-r--r--r--   1        0        0          0 2017-01-05 00:00:00 INFO/REQUIRES
+-r--r--r--   1        0        0          0 2017-01-05 00:00:00 INFO/OBSOLETES
+-r--r--r--   1        0        0          0 2017-01-05 00:00:00 INFO/PROVIDES
 -r--r--r--   1        0        0          0 2017-01-05 00:00:00 INFO/CHANGELOG

comment:2 Changed 3 years ago by andrew_b

  • Component changed from mc-core to tests

comment:3 follow-up: ↓ 8 Changed 3 years ago by mooffie

  • Owner set to mooffie
  • Status changed from new to accepted

It's a surprisingly uninformative message.


It gives everything one needs to know to start investigating the faulty helper: the paths to the output files ("expected" and "actual"). I assumed the diff might be big so I didn't want to provide it outright (I could/can do this: it means adding just one line of code).

Expected output (after parsing): .../data/rpm.custom.output


There are two tests for rpm (rpm.custom.input and rpm.glib.output). Is it really only one of them that fails, or both?

+-r--r--r-- 1 0 0 0 2017-01-05 00:00:00 INFO/PACKAGER
+-r--r--r-- 1 0 0 0 2017-01-05 00:00:00 INFO/URL
+-r--r--r-- 1 0 0 0 2017-01-05 00:00:00 INFO/EPOCH
...


Hmmm.. these specific fields are handled by a sed script. I'll investigate this soon.

(Anybody knows how to install a POSIX-compliant sed on Ubuntu? I'll probably figure this out by googling, but in case anybody knows...)

comment:4 Changed 3 years ago by mooffie

I've found the problem. I'll update you later.

(The problem is the weird way Debian's Dash interprets escape sequences (\a, \t, etc). One can replicate @slyfox's output by changing the rpm helper's shebang from "/bin/sh" to "/bin/bash".)

comment:5 Changed 3 years ago by zaytsev-work

Just FYI, the report is basing on the latest Gentoo, but apparently they also use dash as the default shell.

comment:6 Changed 3 years ago by mooffie

I have the branch ready, but I want to review it further before I push it. It will be soon.

BTW, the accusation in the mailing list, "These tests look fragile", is baseless: as you'll see, the cause was the use of 'echo', in the rpm helper, in a way which is not portable (hence Dash and Bash behave differently). The test input was intentionally exhaustive, which is why the bug was discovered. So we have here a win, not a lose, for the test framework.

(After we finish with this ticket we'll think about making the tester run any '/bin/sh' helper with multiple shells: dash, bash, old bash, ...)

comment:7 Changed 3 years ago by zaytsev

BTW, the accusation in the mailing list, "These tests look fragile", is baseless

Come on, he just didn't get that the tests don't actually run the tools, but rather parse saved tool outputs, so he was thinking that the test failures are due to output drift between different versions of rpm / u7z, etc. As a distro maintainer, you see stupid upstreams doing this kinds of shit all the time & breaking your integration tests like no tomorrow, so this is really the first thing that comes to mind when you see diverging test outputs, and he didn't mean to offend anyone... just jumped to the conclusions a bit too soon.

So we have here a win, not a lose, for the test framework.

Absolutely, congratulations on the first hit in the wild!

comment:8 in reply to: ↑ 3 Changed 3 years ago by slyfox

Replying to mooffie:

It's a surprisingly uninformative message.


It gives everything one needs to know to start investigating the faulty helper: the paths to the output files ("expected" and "actual"). I assumed the diff might be big so I didn't want to provide it outright (I could/can do this: it means adding just one line of code).

I didn't mean to be rude. My apologies.

TL:DR: i'd like 'make check V=1' to dump whatever it needs for upstream to effectively diagnose the failure so I could just report a bug.

Long story: this bug forced me to wade through many indirections to provide any actionable diff at all.

Let me demonstrate:

  1. run 'make check V=1'
FAIL: run
make[9]: Entering directory '/home/slyfox/dev/git/mc/tests/src/vfs/extfs/helpers-list'
Making all in misc
make[10]: Entering directory '/home/slyfox/dev/git/mc/tests/src/vfs/extfs/helpers-list/misc'
make[10]: Nothing to be done for 'all'.
make[10]: Leaving directory '/home/slyfox/dev/git/mc/tests/src/vfs/extfs/helpers-list/misc'
make[10]: Entering directory '/home/slyfox/dev/git/mc/tests/src/vfs/extfs/helpers-list'
make[10]: Nothing to be done for 'all-am'.
make[10]: Leaving directory '/home/slyfox/dev/git/mc/tests/src/vfs/extfs/helpers-list'
make[9]: Leaving directory '/home/slyfox/dev/git/mc/tests/src/vfs/extfs/helpers-list'
============================================================================
Testsuite summary for /src/vfs/extfs/helpers-list
============================================================================
# TOTAL: 1
# PASS:  0
# SKIP:  0
# XFAIL: 0
# FAIL:  1
# XPASS: 0
# ERROR: 0
============================================================================
See tests/src/vfs/extfs/helpers-list/test-suite.log
============================================================================

Is it helpful? I'm afraid not for me. For me it's not clear that 'run' is an executable (the name is too generic to click). Output suggests to look at the log failure.

  1. Looking inside
FAIL: run
=========

...
Testing /home/slyfox/dev/git/mc/tests/src/vfs/extfs/helpers-list/data/rpm.custom.input
/home/slyfox/dev/git/mc/tests/src/vfs/extfs/helpers-list/data/rpm.custom.output /tmp/rpm.actual-parsed-output.aBHnrIV4 differ: char 42, line 1

ERROR: rpm has produced output that's different than the expected output.

  Expected output (after parsing): /home/slyfox/dev/git/mc/tests/src/vfs/extfs/helpers-list/data/rpm.custom.output
  Actual output (after parsing): /tmp/rpm.actual-parsed-output.aBHnrIV4

This might mean that a bug was introduced into rpm. Or that a bug was fixed.
Please compare the files.

If the actual output is the correct one, just copy the latter file
onto the former (and commit to the git repository).

Tip: invoke this program with '--mcdiff' to automatically launch
mcdiff to visually inspect the diff.

Is it already helpful to report as-is? I would not be able to figure out anything about environment. I don't even understand which "this" tool is supposed to take '--mcdiff' option. After the fact it dawned on me it's the run tool.

'run --mcdiff' also runs a diff per failed test. It's not copy/paste-able as is.

  1. Diff actual files and provide you an output. Should be good enough to provide a report. Not so much what is actually different in my environmaned compared to yours. 3 indirections so far.

And you know what? I actually tried to find how the test works to run actual thing which behaves incorrectly before reporting the bug. I've failed.

  1. Noticed 'run' is a shell script. Tried to run 'bash -x ./run' but it's a one-liner script and I need to patch 'run' to get the trace. I've patched '-x' in there and got at least something. Still don't have a command to run on extfs helper directly.

I assumed it's not the thing you expect every user to do when test fails and reported a bug.

If it is the path you expect people to follow every time test fails
then I suggest writing a wiki page on how to decrypt this sort of errors
and stick a link to tests/src/vfs/extfs/helpers-list/test-suite.log.

comment:9 Changed 3 years ago by mooffie

  • Summary changed from tests/src/vfs/extfs/helpers-list test fails in master and 4.8.19 with unclear messages to extfs: rpm helper doesn't always preserve <backslash> in some fields, making one test fail

@slyfox: I've already decided to add diff to the output. I just didn't have time to announce it in a new ticket (I'm busy solving the current issue). Please don't mention this here as it adds noise to the ticket. I'm changing the title of the ticket to reflect this. If you have other issues/questions with the test framework, please open new ticket(s) -- they'd be welcomed.

(Running 'run' interactively is tailored for people who are working to fix the code, not quite for reporters. I'll make it so that running it non-interactively would be more friendly for reporters (i.e., instead of printing "invoke this program with --mcdiff" it'd actually print the diff).)

Last edited 3 years ago by mooffie (previous) (diff)

comment:10 follow-up: ↓ 11 Changed 3 years ago by zaytsev

Summary changed .. extfs: rpm helper ... making one test fail

Sorry, I didn't get it, so this ticket will only be about rpm thing and you will create others for u7z & urar, or all problems have the same root cause and will be addressed in this ticket?

comment:11 in reply to: ↑ 10 Changed 3 years ago by mooffie

Replying to zaytsev:

Summary changed .. extfs: rpm helper ... making one test fail

Sorry, I didn't get it, so this ticket will only be about rpm thing and you will create others for u7z & urar, or all problems have the same root cause and will be addressed in this ticket?


What "all problems"?

@slyfox: did any other test fail except 'rpm.custom.input'? I don't need you to do 'diff'ing. Just look in that same log file (you may paste it here).

(I know he wrote "And a few others" in the mailing list, but people have a tendency to be dramatic.)

(I'm aware that TAP can make autotools' output more informative ("TOTAL: 12; FAIL: 1"). It's certainly on my TODO list.)

comment:12 follow-up: ↓ 14 Changed 3 years ago by zaytsev

What "all problems"?

Just have a look at the complete log file attached to this ticket by slyfox:

https://midnight-commander.org/attachment/ticket/3781/test-suite.log

I see at least 6 failures there, here I cleaned up the output and pasted it for you below as a summary:

1.

	Testing rpm.custom.input
	rpm.custom.output rpm.actual-parsed-output.5KFukg2S differ: char 42, line 1
	ERROR: rpm has produced output that's different than the expected output

2.

	Testing u7z.complex.input
	u7z.complex.output u7z.actual-parsed-output.32c658I0 differ: char 38, line 1
	ERROR: u7z has produced output that's different than the expected output.

3.

	Testing u7z.missing-size-and-date.input
	u7z.missing-size-and-date.output u7z.actual-parsed-output.poUYeOtk differ: char 1, line 1
	ERROR: u7z has produced output that's different than the expected output.

4.

	Testing u7z.simple.input
	u7z.simple.output u7z.actual-parsed-output.ncyZjvRn differ: char 1617, line 23
	ERROR: u7z has produced output that's different than the expected output.	

5.

	Testing urar.v4,v3.input
	urar.v4,v3.output urar.actual-parsed-output.JDADkOlX differ: char 170, line 3
	ERROR: urar has produced output that's different than the expected output.

6.

	Testing urar.v5.input
	urar.v5.output urar.actual-parsed-output.zkYM0qIt differ: char 170, line 3
	ERROR: urar has produced output that's different than the expected output.

comment:13 Changed 3 years ago by zaytsev

@mooffie, I'm sorry for spamming this ticket, but I'd really wish you could be a bit nicer to distro maintainers in the future (of which slyfox is one; he's maintaining the Gentoo ebuild along with polynomial-c). I myself learned to be indulgent towards distro people after becoming a package maintainer myself for a few years. In reality they are the good guys, and they are on our side, keeping evil users away from pesting us with bad bug reports, and patching packages with backported master commits so that we don't get tons of duplicate reports for bugs which are already fixed between the release cycles... Also, they have a very special perspective on things due to the way they work, and if you put yourself in their shoes, you will see their reports in a different light:

  • Most often they update the packages locally, send them to the build farm of some sort and get back the build logs. If there is a problem, they are very happy if they can get all needed info in copy & paste form ready there, because accessing the builders is often difficult or impossible, and this means extra effort to reproduce and investigate the problem. This is where asking for diffs to be included comes from, for instance.
  • On top of that, they tend to update ("bump") many packages per day, and for them a new release is not some special occasion, but rather everyday routine, so if they notice problems with the new version, they try their best to extract what they see as useful information from the build log, submit it upstream ASAP and keep processing their package queue. They often see typical mistakes by upstreams and identify problems by just looking at the logs, although sometimes this leads to misattributions. But they always mean well.

I hope this clarifies the situation a little bit, I really don't want to moralize, but rather provide helpful input to interpret the communications around this problem.

Now, I think, we already have a wish list which we can discuss in separate tickets / on the mailing list, but I'll leave it here as a summary:

  • Rename run to something like run-extfs-helpers-tests (to be discussed?)
    • Is there an easy way to re-run single test / failed tests?
  • Revise output on failure, include diffs (already planned?)
    • If not by default, then e.g. when re-running single individual tests?
  • Provide better summary through TAP integration (already planned?)

comment:14 in reply to: ↑ 12 Changed 3 years ago by mooffie

Replying to zaytsev:

What "all problems"?

Just have a look at the complete log file attached to this ticket by slyfox:


Woa! I didn't notice the attachment.

Omigosh.

I'll start working on #3784 right away.

Replying to zaytsev:

I'd really wish you could be a bit nicer to distro maintainers in the future


If you got the impression I was rude, that was only an impression. I appologize to anybody if it seemed so.

he's maintaining the Gentoo ebuild


BTW, @slyfox: if there a chance you could give me temporary SSH access to some Gentoo system? I could then easily solve problems (diff output is nice, but one can't always deduce the problem from it). I know there are VM softwares but they don't work on my ancient machine.

In reality they are the good guys, and they are on our side, keeping evil users away from pesting us with bad bug reports, and patching packages with backported master commits so that we don't get tons of duplicate reports for bugs which are already fixed between the release cycles... Also, they have a very special perspective on things due to the way they work, and if you put yourself in their shoes, you will see their reports in a different light


I agree.

Revise output on failure, include diffs (already planned?)


Sure. I opened a ticket for this (before I saw your new comment). #3784

Provide better summary through TAP integration (already planned?)


Yes, TAP is planned. In the past days I've been busy looking into some "panelization" related bugs, which is where my time went to -- I'll postpone this.

Is there an easy way to re-run single test / failed tests?


I planned this together with the TAP refactoring.

comment:15 Changed 3 years ago by mooffie

@slyfox: I left you a message in #3784 explaining how to update the tester (to include the diff in the output). It would be great if you could try it out: after updating it run "make check" and attach the new test-suite.log here again.

comment:16 Changed 3 years ago by mooffie

  • Summary changed from extfs: rpm helper doesn't always preserve <backslash> in some fields, making one test fail to extfs: Some tests fail on Gentoo. Reason(s) yet unknown.

comment:17 follow-up: ↓ 18 Changed 3 years ago by mooffie

Based on the log, here's a summary of the tests that pass/fail on Gentoo:

hp48+.input                        PASSED
lslR.1.spaces.input                PASSED
lslR.2.spaces-iso.input            PASSED
lslR.3.spaces-iso-noslash.input    PASSED
rpm.custom.input                   **ERROR**  (char 42, line 1)
rpm.glib.input                     PASSED
u7z.complex.input                  **ERROR**  (char 38, line 1)
u7z.missing-date.input             PASSED
u7z.missing-size-and-date.input	   **ERROR**  (char 1, line 1)
u7z.simple.input                   **ERROR**  (char 1617, line 23)
uace.input                         PASSED
uarc.input                         PASSED
urar.v4,v3.input                   **ERROR**  (char 170, line 3)
urar.v5.input                      **ERROR**  (char 170, line 3)
uzip.with-zipinfo.input            PASSED
uzip.without-zipinfo--mdy.input    PASSED
uzip.without-zipinfo--ymd.input    PASSED
uzoo.input                         PASSED

I know what problem caused 'rpm.custom.input' to fail, but I don't see how it's related to the other failed tests.

Interesting: the helpers of the failed tests use the 'sort' utility; the helpers of the passed tests don't. Maybe that's the problem, maybe not.

Replying to mooffie:

BTW, @slyfox: is there a chance you could give me temporary SSH access to some Gentoo system?

Would be nice to try it out on a *BSD account too (as their tools aren't elastic as GNU's). I think I'll install another hard disk in my computer and try it out (it'd be easier than clearing up space on my small disk; I have plenty of old disks in my closet). If you suddenly don't hear from me, know that I've messed up something ;-) Or is there some other option?

comment:18 in reply to: ↑ 17 Changed 3 years ago by mooffie

Replying to mooffie:

Interesting: the helpers of the failed tests use the 'sort' utility; the helpers of the passed tests don't. Maybe that's the problem, maybe not.


That's indeed the problem -- at least for the u7z helper: @Sergey sent a diff to the mailing list:

https://mail.gnome.org/archives/mc-devel/2017-March/msg00004.html

You can see that 'sort' orders filenames beginning with "$" first. Our "expected output" has it sorted differently.

The test for urar most probably fails for the same reason: some filenames in it start with ".".

(For some reason I got @Sergey's post in my gmail box only "lately": much after @Yury's reply to it. So I wasn't aware of it.)

comment:19 follow-up: ↓ 20 Changed 3 years ago by mooffie

Ok, lets decide what to do:

  • I opened a new ticket, #3786, for the rpm test bug (which is not related to 'sort').
  • This ticket could be renamed to "extfs: urar and u7z helpers use of 'sort' makes tests fail". Or we could start a new ticket by that name and make this one blocked by it, or close it. @Yury: what do you prefer?

comment:20 in reply to: ↑ 19 Changed 3 years ago by mooffie

  • Blocked By 3786, 3787 added

Replying to mooffie:

Ok, lets decide what to do:

  • This ticket could be renamed [...] Or we could start a new ticket by that name and make this one blocked by it [...] @Yury: what do you prefer?


Ok, I decided it for you: since we don't have @slyfox available for us 24/7, I started a new ticket for the 'sort' issue.

I'm making this ticket blocked by the two new tickets. When @slyfox tells us all is fixed we'll close it.

comment:21 Changed 3 years ago by zaytsev

@mooffie, oh shi~! It seems that you have identified the problem. I also faced the same thing when writing integration tests for the compiler I mentioned once, and sorting output data in a tabular format (produced by different threads) only by a subset of columns.

The problem is, when one uses sort -k, nothing says it will be a stable sort, so otherwise equivalent lines may come in random order depending on sort implementation and the phase of the moon. As far as I know, -s argument (for stable sort) is an unportable GNU extension. At that time I chose to use it, because it was the easiest solution, and we were sure the software will not exist outside of our Linux-based distribution. Here, however, this doesn't sound like an option so some further thinking is needed to find a way out. Of course, we can just declare that we ignore the order for tests and fully sort the output, but this somehow doesn't sound great.

Just thought this would be helpful to know...

comment:22 Changed 3 years ago by zaytsev

Oh, so it was locale related in the end and had nothing to do with sort -k 8, sorry I've just woke up :-/ I will leave the comment as is though in order to not cause further confusion.

comment:23 follow-up: ↓ 24 Changed 3 years ago by slyfox

I've merged 2 branches to test a change:

$ git merge origin/3784_extfs_tester_diff origin/3787_extfs_tester_sort_locale
$ ./autogen.sh
$ ./configure
$ LANG=en_US.UTF-8 make check

Most of tests are now passing. rpm one still fails:

ERROR: rpm has produced output that's different than the expected output.

  Expected output (after parsing): /home/slyfox/dev/git/mc/tests/src/vfs/extfs/helpers-list/data/rpm.custom.output
  Actual output (after parsing): /tmp/rpm.actual-parsed-output.kLHkyHWQ

This might mean that a bug was introduced into rpm. Or that a bug was fixed.
Please compare the files.

If the actual output is the correct one, just copy the latter file
onto the former (and commit to the git repository).

------------ diff of the expected output vs the actual output: -------------
--- /home/slyfox/dev/git/mc/tests/src/vfs/extfs/helpers-list/data/rpm.custom.output     2017-01-22 16:42:40.730438199 +0000
+++ /tmp/rpm.actual-parsed-output.kLHkyHWQ      2017-03-08 08:16:57.575208078 +0000
@@ -1,3 +1,3 @@
--r--r--r--   1        0        0        539 2017-01-05 00:00:00 HEADER
+-r--r--r--   1        0        0        597 2017-01-05 00:00:00 HEADER
 -r-xr-xr-x   1        0        0         39 2017-01-05 00:00:00 INSTALL
 -r-xr-xr-x   1        0        0         39 2017-01-05 00:00:00 UPGRADE
@@ -14,4 +14,5 @@
 -r--r--r--   1        0        0          0 2017-01-05 00:00:00 INFO/VENDOR
 -r--r--r--   1        0        0          0 2017-01-05 00:00:00 INFO/DESCRIPTION
+-r--r--r--   1        0        0          0 2017-01-05 00:00:00 INFO/SUMMARY
 -r--r--r--   1        0        0          0 2017-01-05 00:00:00 INFO/SCRIPTS/PRETRANS
 -r--r--r--   1        0        0          0 2017-01-05 00:00:00 INFO/SCRIPTS/POSTTRANS
@@ -29,4 +30,11 @@
 -r--r--r--   1        0        0          0 2017-01-05 00:00:00 INFO/SCRIPTS/POSTUNPROG
 -r--r--r--   1        0        0          0 2017-01-05 00:00:00 INFO/SCRIPTS/VERIFYSCRIPTPROG
+-r--r--r--   1        0        0          0 2017-01-05 00:00:00 INFO/PACKAGER
+-r--r--r--   1        0        0          0 2017-01-05 00:00:00 INFO/URL
+-r--r--r--   1        0        0          0 2017-01-05 00:00:00 INFO/EPOCH
+-r--r--r--   1        0        0          0 2017-01-05 00:00:00 INFO/LICENSE
+-r--r--r--   1        0        0          0 2017-01-05 00:00:00 INFO/REQUIRES
+-r--r--r--   1        0        0          0 2017-01-05 00:00:00 INFO/OBSOLETES
+-r--r--r--   1        0        0          0 2017-01-05 00:00:00 INFO/PROVIDES
 -r--r--r--   1        0        0          0 2017-01-05 00:00:00 INFO/CHANGELOG
 -r--r--r--   1        0        0          0 2017-01-05 00:00:00 CONTENTS.cpio
------------------------------- end of diff --------------------------------

My environment is:

/bin/sh: GNU bash, version 4.4.12(1)-release 
/bin/grep: grep (GNU grep) 2.27
/bin/awk: GNU Awk 4.1.3, API: 1.1
/bin/sed: (GNU sed) 4.2.2

Changed 3 years ago by slyfox

updated test log

comment:24 in reply to: ↑ 23 Changed 3 years ago by andrew_b

Replying to slyfox:

ERROR: rpm has produced output that's different than the expected output.

I have the same exactly: ticket:3786#comment:3.

comment:25 Changed 3 years ago by slyfox

I guess HEADER size instability comes from echo usage on external data
which comest from Description: part:

Description :
Multi-line description field
with "double", 'single quotes', and $weird | \characters i\n = i\\t, empty line...

...and a tab: [ ].'
  • src/vfs/extfs/helpers/rpm

    a b mcrpmfs_list () 
    224224    DATE=`mcrpmfs_getRawOneTag "%{BUILDTIME:date}\n" | cut -c 5-11,21-24` 
    225225    PAYLOAD=`mcrpmfs_getRawOneTag "%{PAYLOADFORMAT}\n" | sed s/ustar/tar/` 
    226226 
    227     HEADERSIZE=`echo "$DESC" | wc -c` 
     227    HEADERSIZE=`printf '%s' "$DESC" | wc -c` 
    228228    printf '%s %s %s HEADER\n' "${FILEPREF}"  "${HEADERSIZE}" "${DATE}" 
    229229    echo "-r-xr-xr-x   1 root     root    39 $DATE INSTALL" 
    230230    case "${rpm_filename}" in 

While looking at echo call sites I've noticed suspicious grep for a constant string.
Likely a missing '$' was intended.

  • src/vfs/extfs/helpers/rpm

    a b mcrpmfs_getDesription() 
    8989mcrpmfs_getAllNeededTags() 
    9090{ 
    9191    supportedTags=`mcrpmfs_getSupportedTags` 
    92     if test "`echo supportedTags | grep -c CONFLICTS`" -eq 1; then 
     92    if test "`echo $supportedTags | grep -c CONFLICTS`" -eq 1; then 
    9393       tag_CONFLICTS="|CONFLICTS=%{CONFLICTS}" 
    9494    else 
    9595       tag_CONFLICTS="" 

comment:26 Changed 3 years ago by mooffie

Replying to zaytsev:

The problem is, when one uses sort -k, nothing says it will be a stable sort, so otherwise equivalent lines may come in random order depending on sort implementation
[...]
Oh, so it was locale related in the end and had nothing to do with sort -k 8, sorry


(The problem couldn't be related to non-stable sort because that would mean the helper reports a pathname twice. This would be a bug in the helper. But it's good you've mentioned this, it's certainly something to keep in mind!)

Replying to slyfox:

I've merged 2 branches to test a change:
[...]
Most of tests are now passing. rpm one still fails:


Yes, I know. I explained that the rpm bug isn't related to 'sort'. It's handled in #3786.

Replying to slyfox:

I guess HEADER size instability comes from echo usage on external data


Right. I wrote in comment:6 that echo is the cause (quite interesting, isn't it? that was new to me). As I mentioned, I already have a fixed branch ready; the reason I haven't yet pushed it is because I'm undecided about something in it (I'll share my dilemma with you folks in #3786 in a couple of hours I hope).

Replying to slyfox:

While looking at echo call sites I've noticed suspicious grep for a constant string.
Likely a missing '$' was intended.


Yep, I'm aware of this. I'll later open a ticket about this (and one another) bug.

Slyfox, thank you for your cooperation in reporting. It's much appreciated!

comment:27 Changed 3 years ago by mooffie

I've re-read all comments in this ticket.

I believe all issues mentioned in asides are already handled in other tickets:

  • The bad user experience (UX) described in comment:8 is handled by #3784 or #3785.
  • The missing "$" bug is handled in #3789.

(@slyfox: thank you, again, for your cooperation. @Yury: thank you for making me aware of some important things in comment:13.)

Last edited 3 years ago by mooffie (previous) (diff)

comment:28 Changed 3 years ago by mooffie

Now that both #3786 & #3787 are fixed we can close this ticket. I'll leave this to @slyfox, once he sees all is alright. (If he doesn't come back we'll close it ourselves in a couple of days.) There's no need to add anything to NEWS-4.8.20: it's already done.

comment:29 Changed 3 years ago by slyfox

  • Status changed from accepted to testing
  • Resolution set to fixed

master now passes all vfs tests for me.

Thank you!

comment:30 Changed 3 years ago by andrew_b

  • Status changed from testing to closed
  • Milestone changed from Future Releases to 4.8.20
Note: See TracTickets for help on using tickets.