Ticket #3729 (closed defect: fixed)

Opened 11 months ago

Last modified 10 months ago

Fix three VFSs that use obolete dates: hp48, uace, uarc.

Reported by: mooffie Owned by: zaytsev
Priority: minor Milestone: 4.8.19
Component: mc-vfs Version: master
Keywords: Cc:
Blocked By: #3730, #3744 Blocking:
Branch state: merged Votes for changeset: committed-master

Description

(Forking from #3696.)

These VFSs use the "Mon DD YYYY hh:mm" format, which since 2006 is no longer supported.

Change History

comment:1 Changed 11 months ago by mooffie

  • Blocked By 3730 added

It will be very easy to fix these once we have #3730 in.

comment:2 Changed 10 months ago by mooffie

(Just wanted to say that I've had the patches ready for a couple of days. I'll post them soon. The tester in #3730 indeed makes things easy.)

comment:3 Changed 10 months ago by mooffie

  • Blocked By 3744 added


I'm marking this "blocked by #3744". The tickets are unrelated but both touch Makefile.am and will therefore result in merge conflict unless ordered.

comment:4 Changed 10 months ago by mooffie

Here are the patches. They include tests.

But wait till #3744 gets committed.

Notes:

  • You don't need to wait 5 years for somebody to review these patches. Those days are over. You can commit them asap: the accompanying tests will tell you if everything is ok.
  • The patches also fix a few other minor bugs in the helpers. It was a necessity: It's not possible to write tests without first making the helpers work.
  • (The commit messages begin with "Ticket #3729:" only in the 3 patches that fix the date format.)

Changed 10 months ago by mooffie

Changed 10 months ago by mooffie

Changed 10 months ago by mooffie

Changed 10 months ago by mooffie

Changed 10 months ago by mooffie

Changed 10 months ago by mooffie

Changed 10 months ago by mooffie

Changed 10 months ago by mooffie

Changed 10 months ago by mooffie

Changed 10 months ago by mooffie

comment:5 Changed 10 months ago by zaytsev

  • Owner set to zaytsev
  • Status changed from new to accepted
  • Milestone changed from Future Releases to 4.8.19

comment:6 Changed 10 months ago by zaytsev

  • Branch state changed from no branch to on review

Branch: 3729_vfs_date_fixes
Initial changeset: [9dac3f2669c8f3f3f1be302a3bfe7c536bec4731].

Awesome, I'm very impressed, especially by the hp48 fixes :-)

Last edited 10 months ago by andrew_b (previous) (diff)

comment:7 Changed 10 months ago by zaytsev

Looking at the Travis build log:

Testing /home/travis/build/MidnightCommander/mc/distrib/mc-f1ef3702948950b65da87ef8442a6dc2e6f52ac2/build-default/../tests/src/vfs/extfs/helpers-list/data/u7z.complex.input

/home/travis/build/MidnightCommander/mc/distrib/mc-f1ef3702948950b65da87ef8442a6dc2e6f52ac2/build-default/../tests/src/vfs/extfs/helpers-list/data/u7z.complex.output /tmp/u7z.actual-parsed-output.UZf4llQ9 differ: byte 1908, line 21

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

...

w00t? Here is the answer:

tar: mc-f1ef3702948950b65da87ef8442a6dc2e6f52ac2/tests/src/vfs/extfs/helpers-list/data/u7z.missing-size-and-date.env_vars: file name is too long (max 99); not dumped

Not nice :-( time to switch to at least tar-ustar? First need to fix #3603 though. In the mean time, trying to work around it by adding --always to git describe.

comment:8 Changed 10 months ago by andrew_b

  • Votes for changeset set to andrew_b
  • Branch state changed from on review to approved

comment:9 Changed 10 months ago by zaytsev

  • Status changed from accepted to testing
  • Votes for changeset changed from andrew_b to committed-master
  • Resolution set to fixed
  • Branch state changed from approved to merged

comment:10 Changed 10 months ago by zaytsev

  • Status changed from testing to closed

comment:11 follow-up: ↓ 12 Changed 10 months ago by andrew_b

Well, I got an error in hp48+ test:

--- hp48+.actual-parsed-output.hDRkf1Mr	2016-12-27 10:54:57.459848035 +0300
+++ hp48+.actual-output.yAx9c3Ry	2016-12-27 10:54:57.457848045 +0300
@@ -1,3 +1,3 @@
--rw-r--r--   1        0        0          0 /YEN
--rw-r--r--   1        0        0          0 /JYTLIGHT
--rw-r--r--   1        0        0          0 /IOPAR
+-rw-r--r--   1 0        0               0 12-27-2016 10:54 /YEN
+-rw-r--r--   1 0        0               0 12-27-2016 10:54 /JYTLIGHT
+-rw-r--r--   1 0        0               0 12-27-2016 10:54 /IOPAR

comment:12 in reply to: ↑ 11 ; follow-up: ↓ 13 Changed 10 months ago by mooffie

Replying to andrew_b:

Well, I got an error in hp48+ test:

+-rw-r--r--   1 0        0               0 12-27-2016 10:54 /YEN
+-rw-r--r--   1 0        0               0 12-27-2016 10:54 /JYTLIGHT
+-rw-r--r--   1 0        0               0 12-27-2016 10:54 /IOPAR


The filesize column in your output is all zero.

The helper converts fractional file sizes ("30.5", "21848.5", etc.) into integer ones ("30", "21848", etc.) using a printf trick which may not be portable and which may be the reason all the filesizes in your output are zero. (Don't ask me why the filesizes the calculator reports have fractions: I don't know.)

What's the output of the following command on your system?

$ /bin/sh -c 'printf %d 30.5 2>/dev/null'

(The purpose of the "/bin/sh" is to use the builtin (or otherwise) printf of the shell which will actually run the helper, and not the builtin printf of your interactive shell.)

If nothing is printed: what kind of shell is /bin/sh on your system? Now remove the "2>/dev/null" part and tell us what it says. I'll soon check the standard to see if that printf trick is portable.

comment:13 in reply to: ↑ 12 Changed 10 months ago by andrew_b

Replying to mooffie:

If nothing is printed: what kind of shell is /bin/sh on your system?

$ sh --version
GNU bash, version 3.2.57(1)-release (x86_64-alt-linux-gnu)
Copyright (C) 2007 Free Software Foundation, Inc.

/bin/sh is the independent executable file, the restricted version of bash. It is built specially for non-interactive usage.

Now remove the "2>/dev/null" part and tell us what it says.

$ /bin/sh -c 'printf %d 30.5 2>/dev/null'
0

$ LC_ALL=C /bin/sh -c 'printf %d 30.5'
/bin/sh: line 0: printf: 30.5: invalid number

But if I use %f instead of %d, I have the result that seems correct:

$ LC_ALL=C /bin/sh -c 'printf %.0f 30.5'
30

$ LC_ALL=C /bin/sh -c 'printf %.1f 30.5'
30.5

$ LC_ALL=C /bin/sh -c 'printf %.2f 30.5'
30.50

comment:14 Changed 10 months ago by mooffie

Thanks for the information.

I opened a new ticket for this problem: #3747.

Note: See TracTickets for help on using tickets.