Prepare for release mc-4.8.16

[zaytsev]

See ReleaseGuidelines for more details.

[slavazanko]

JFYI: The branch 3547_cleanup has been created for the ticket

[zaytsev-work]

FIXFOR Ticket #3547: move subshell stuff into subdir.

src/subshell/common.c

-Vit Rosin <vit_r@…
+Vit Rosin <vit_r@…>

[andrew_b]

(In #3555) No reason for special ticket. Use #3547 instead.

[and]

next cleanup patches attached, let me know if this ticket is right for cleanup patch requests

[zaytsev]

This ticket is the right one, thanks!

[andrew_b]

mc-3547-cleanup-conversion-warning-lock_c.patch: applied with additional changes.

mc-3547-cleanup-conversion-warning-keybind_t.patch
I'd prefer to make command of (signed) long type not int.

mc-3547-cleanup-conversion-warning-strutils_escape.patch

remove src_len parameter support, never used in mc

The another way should be found. I think we shouldn't introduce any limitation in library code. Currently source string length is not used, but who knows about future?

mc-3547-cleanup-conversion-warning-mc_search_new.patch: applied with some changes.

[andrew_b]

mc-3547-cleanup-conversion-warning-gauge_c.patch: applied.

mc-3547-cleanup-conversion-warning-menubar_selected.patch: WMenubar related part is applied, editor menu is fixed with another way.

Andreas, it would be great if you create your patches using git format-patch command. Thanks!

[zaytsev]

Trivial doc patch in #3556 can be committed upon review.

[zaytsev]

Check that one can do update-po on the tarballs before the next release (#3553).

[andrew_b]

mc-3547-cleanup-conversion-warning-chown_advanced_but.patch: applied
mc-3547-cleanup-conversion-warning-chown_but.patch​: applied
mc-3547-cleanup-conversion-warning-chmod_but.patch​: applied
mc-3547-cleanup-conversion-warning-mode_t.patch: applied
mc-3547-remove-outdated-comment-about-refresh.patch​: applied
mc-3547-fix-compiler-warning-at-search.c.patch: applied

[andrew_b]

Replying to andrew_b:

mc-3547-cleanup-conversion-warning-keybind_t.patch
I'd prefer to make command of (signed) long type not int.

Done.

[andrew_b]

mc-3547-listbox.c-remove-redundant-code.patch​: applied.

[mooffie]

@Andreas:

I'm curious about how you work: let's say you've just edited the source code to fix some compiler warnings -- what are you doing next?

[and]

Replying to mooffie:

@Andreas:

I'm curious about how you work: let's say you've just edited the source code to fix some compiler warnings -- what are you doing next?

Is this a criticism (which intention) or a question about work flow? :)

If we don't care about "valid" compiler warnings, we are not worthy for portable code.

Further, let's assume you are a package maintainer at a distribution and wan't annex mc. First thing you do are pedantic compile runs to get a basic view about code quality

I like to see solving -Wformat-nonliteral completely
maybe full -Wconversion fixing is not possible because of gnome inconsistent

finding/fixing memleaks should be valuable, you can help out while running mc with
-fsanitize=*

[zaytsev]

I think it was simply a question about the workflow...

Replying to mooffie:

I'm curious about how you work: let's say you've just edited the source code to fix some compiler warnings -- what are you doing next?

We (just as many others) have an --enable-maintainer-mode option, which, among other things, turns on -Werror. Andreas is adding the warning flags to our CFLAGS after fixing the warnings, so the build will break if they come back again.

[mooffie]

Replying to and:

If we don't care about "valid" compiler warnings, we are not worthy for portable code.

I wholly agree.

Is this a criticism (which intention) or a question about work flow? :)

Mmmm... a sincere question. I wanted to know to what lengths you go to test your modifications afterwards. E.g., if, say, a patch that changes signedness in 30 files requires us to check that each and every touched component continues to work afterwards. Me, for example, being inflicted by "compulsive behavior", wouldn't dare embarking on such a patch because it'd mean I'd have to eat my dinner cold --which is why I greatly appreciate your doing this.

finding/fixing memleaks should be valuable, you can
help out while running mc with -fsanitize=*

Yeah, these are certainly outright bugs. (It's not very practical on my slow computer to run mc with runtime checks, unfortunately.)

maybe full -Wconversion fixing is not possible because of [...]

Hmmm, speaking of which, take a look at the two patches with "chown" in their names. They introduce a bug: they change the order of variables but the initializers afterwards assume the old order. I.e., NORMAL_BUTTON is assigned to 'y', not to 'flags'. I wonder why -Wsign-conversion didn't flag this initialization. Wouldn't it mean assigning unsigned to signed? Whatever. Could you please provide a patch to fix this?

Replying to zaytsev:

We (just as many others) have an --enable-maintainer-mode option, which, [...] Andreas is adding the warning flags to our CFLAGS after fixing the warnings

Thank you for explaining.

[zaytsev-work]

Replying to mooffie:

it'd mean I'd have to eat my dinner cold

He eats his dinner cold anyways, it's called Abendbrot ;-)

finding/fixing memleaks should be valuable, you can
help out while running mc with -fsanitize=*

Yeah, these are certainly outright bugs. (It's not very practical on my slow computer to run mc with runtime checks, unfortunately.)

Google sanitizers are orders of magnitude faster than valgrind, you should definitively check them out.

[andrew_b]

Replying to mooffie:

Hmmm, speaking of which, take a look at the two patches with "chown" in their names. They introduce a bug: they change the order of variables but the initializers afterwards assume the old order. I.e., NORMAL_BUTTON is assigned to 'y', not to 'flags'.

I fixed this.

[and]

when testing branch 3547_cleanup I get this at src/filemanager/panel.c

panel.c:111:15: error: no previous extern declaration for non-static variable 'panel_fields' [-Werror,-Wmissing-variable-declarations]
panel_field_t panel_fields[] = {
              ^

is branch 3547_cleanup inappropriate for testing?

[andrew_b]

Replying to and:

when testing branch 3547_cleanup I get this at src/filemanager/panel.c

panel.c:111:15: error: no previous extern declaration for non-static variable 'panel_fields' [-Werror,-Wmissing-variable-declarations]
panel_field_t panel_fields[] = {
              ^

Fixed.

is branch 3547_cleanup inappropriate for testing?

Cleanup branch can be tested in ant time.

[andrew_b]

(In #3567) mc-3567-fix-heap-use-after-free-bug-widget-object-v6.patch​: applied with renamed variable.

[andrew_b]

Stop please. As I wrote in ticket:3473#comment:4, no reason to fix valid code. See ​https://fedoraproject.org/wiki/Format-Security-FAQ

[and]

ok I understand, I will prepare another new memleak fix. :)

[and]

please double test fish section [read() replaced by lseek()]

[and]

what more preferred? using bit fields and saving bytes or using gboolean for FALSE/TRUE state values

[mooffie]

Invalid command-line options cause segfault.

E.g., run mc with "mc --hell" (instead of "mc --help") to see this.

I'm attaching a patch to fix this. The problem was introduced in this branch (3547_cleanup) so I'm not opening a new ticket.

[andrew_b]

Replying to mooffie:

Invalid command-line options cause segfault.

E.g., run mc with "mc --hell" (instead of "mc --help") to see this.

I'm attaching a patch to fix this. The problem was introduced in this branch (3547_cleanup) so I'm not opening a new ticket.

Thanks! This patch must be squashed with commit were bug was appeared in.

[andrew_b]

mc-3547-treestore.c-cleanup-bit-fields-warnings.patch​: gboolean flags are used instead of bit fields.

mc-3547-cleanup-gcc-link-time-optimization-warnings.patch​: parts of variable initialization were applied. Did you test optimization of fish yourself?

[andrew_b]

So, now we have about 40 commits in the 3547_cleanup branch. I think it's time to review it and merge into master.

Branch was rebased to current master.
Initial changeset:da467f25e321421c8035381d05cdb406d67f1980
Please review.

[and]

Replying to andrew_b:

mc-3547-treestore.c-cleanup-bit-fields-warnings.patch​: gboolean flags are used instead of bit fields.

mc-3547-cleanup-gcc-link-time-optimization-warnings.patch​: parts of variable initialization were applied. Did you test optimization of fish yourself?

It's pointed out, seeking with a nonzero position is not supported on sockets
I will attach a new patch with normal read syscall

[and]

tested with fish shell and large file transfer abort

[andrew_b]

mc-3547-fish.c-cleanup-gcc-link-time-optimization-warning.patch

sizeof *buf

Unneeded. In C, sizeof (char) == 1.

                   g_free(buf);
                   bufsize = (size_t) (fish->total - fish->got);
	           buf = g_malloc (bufsize * sizeof *buf);

Try g_realloc() or better g_try_realloc(). Btw, how much size of memory allocation you expect here?

[and]

Replying to andrew_b:

mc-3547-fish.c-cleanup-gcc-link-time-optimization-warning.patch

sizeof *buf

Unneeded. In C, sizeof (char) == 1.

                   g_free(buf);
                   bufsize = (size_t) (fish->total - fish->got);
	           buf = g_malloc (bufsize * sizeof *buf);

Try g_realloc() or better g_try_realloc(). Btw, how much size of memory allocation you expect here?

(fish->total - fish->got) holds initial the size of currently dealing file via fish shell, working with a contant buffer is better than alloc gigabyte of memory which we wasting/abort later. This is the current approach also.

[and]

reduce possible future coding faults, binary checksum not changes but this patch

[and]

Replying to andrew_b:

mc-3547-fish.c-cleanup-gcc-link-time-optimization-warning.patch

sizeof *buf

Unneeded. In C, sizeof (char) == 1.

                   g_free(buf);
                   bufsize = (size_t) (fish->total - fish->got);
	           buf = g_malloc (bufsize * sizeof *buf);

Try g_realloc() or better g_try_realloc(). Btw, how much size of memory allocation you expect here?

v2 patch with your suggestion g_try_realloc() and without bufsize helper value.

When fish transfer aborted by user, fish file data will read (for discard) in BUF_8K chunks until last chunk which has size of last remaining filedata.

[andrew_b]

Replying to and:

v2 patch with your suggestion g_try_realloc() and without bufsize helper value.

char *buf;
buf = g_malloc (BUF_8K);
if (sizeof (*buf) > (size_t) (fish->total - fish->got))

This is wrong. sizeof(*buf) will return you the size of pointer (4 or 8 bytes) not the size of allocated memory chunk.

[andrew_b]

Replying to andrew_b:

This is wrong. sizeof(*buf) will return you the size of pointer (4 or 8 bytes) not the size of allocated memory chunk.

Sorry. sizeof(buf) returns the size of pointer. sizeof(*buf) returns 1.

[andrew_b]

mc-3547-robust-sizeof-usage-at-function-parameter-and-use-me.patch​: applied with some modifications.
mc-3547-diffviewer.c-use-gboolean-at-WDiff-struct.patch​: applied.
mc-3547-cleanup-function-declare-at-USE_DIFF_VIEW.patch​: applied.
mc-3547-remove-unused-const-INVALID_OFFSET.patch​: applied.
mc-3547-remove-unused-function-exec_shell.patch​: applied.

[andrew_b]

Replying to andrew_b:

So, now we have about 40 commits in the 3547_cleanup branch. I think it's time to review it and merge into master.

Two weeks of silence.

Merged to master: [d58ed4987a8d5b642c2aaccf9d7a4bf0f63c6877].

git log --pretty=oneline 24d09ba..d58ed49

[and]

Replying to andrew_b:

Replying to andrew_b:

This is wrong. sizeof(*buf) will return you the size of pointer (4 or 8 bytes) not the size of allocated memory chunk.

Sorry. sizeof(buf) returns the size of pointer. sizeof(*buf) returns 1.

I was too greedy for bufsize value removal -.- and testing on a fast machine don't show performance penalty for 1-byte chunk loop.

[andrew_b]

mc-3547-clarify-edit_file-and-edit_stack_-declares.patch​: this is pointless because src/editor isn't compiled when the --without-internal-edit build option is used.

[zaytsev]

Hi Andrew, happy new year!

Thank you for the excellent work. Sorry, I wasn't able to review this heap, but I trust your judgement, and you did it right that you simply merged it after a few weeks notice. I've only now got a small amount of time to check if I can do something quickly for mc :-/

Incidentally, what are your views with respect to making a release? There are quite a lot of commits now, but nothing major. In a couple of months from now? Having done it once, I can help the next time.

[and]

Replying to andrew_b:

mc-3547-clarify-edit_file-and-edit_stack_-declares.patch​: this is pointless because src/editor isn't compiled when the --without-internal-edit build option is used.

you're right, than we have

macro_index
record_macro_buf
macros_list

that are unneeded exist when using --without-internal-edit

I will make a patch

[andrew_b]

New branch 3547_cleanup is created.
Initial changeset:63a0a93cc61527f1cda53affb26835fb52e001a8

[zaytsev]

Re. extensions, LGTM, thanks! Just to record the command:

cat mcedit.clip | tr ';' '\n' | sort | tr '\n' ';'

[zaytsev]

Rebased against master and force-pushed to stop Travis from complaining about indentation :-/

[andrew_b]

mc-3547-clarify-mc_args__debug_level-declare.patch​: applied.
mc-3547-remove-unused-execute_with_vfs_arg-function.patch​: I couldn't remove currently unused functions. They are can be useful in future.
mc-3547-cleanup-unused-parent_call_string-function.patch​: likewise.
mc-3547-cleanup-internal-filemanager-layout-variables.patch​: applied with some changes: reduce variable scope.
mc-3547-cleanup-unneeded-mc_file-static-cast.patch​: applied with trivial change.
mc-3547-clarify-macro-variables-for-use_internal_edit.patch​: applied.
mc-3547-cleanup-unused-option_edit_-_extreme-variables.patch​: applied.

[and]

mhh,
I vote for MC_ (ALL_CAP) autoconf macros and not for lowercase mc_ macros

​https://www.gnu.org/software/autoconf/manual/autoconf-2.61/html_node/Macro-Names.html

All of the Autoconf macros have all-uppercase names starting with `AC_' to prevent them from accidentally conflicting with other text. All shell variables that they use for internal purposes have mostly-lowercase names starting with `ac_'.

[andrew_b]

Replying to and:

mhh,
I vote for MC_ (ALL_CAP) autoconf macros and not for lowercase mc_ macros

​https://www.gnu.org/software/autoconf/manual/autoconf-2.61/html_node/Macro-Names.html

All of the Autoconf macros have all-uppercase names starting with `AC_' to prevent them from accidentally conflicting with other text. All shell variables that they use for internal purposes have mostly-lowercase names starting with `ac_'.

gnulib macros have lowercase gl_ prefix. coreutils macros have lowercase cu_ prefix. mc_cu_PREREQ_STAT_PROG looks nicely than MC_cu_PREREQ_STAT_PROG.

[mooffie]

Problem: "make dist" doesn't bundle subshell.h. That's because this header file isn't listed in the makefile. I'm attaching a patch.

[mooffie]

(I didn't test this patch, sorry.)

[mooffie]

(By "I didn't test this patch" I didn't mean to say it doesn't work. It's just that testing it would take ages on my system. Writing a patch seemed better than writing a lengthy comment explaining the problem.)

[zaytsev]

LGTM; broken by Slava in 974ab368ec30fcd68e595252668d1542a5352619.

Maybe we should change the Travis scripts to do a make dist, and then use that to do mc builds instead of the vcs checkout...

[andrew_b]

Applied. Thanks!

[zaytsev]

Re. f462265, type is used inside of the condition.

../../../src/viewer/mcviewer.c: In function ‘mcview_load’:
../../../src/viewer/mcviewer.c:380:73: error: ‘type’ undeclared (first use in this function)
../../../src/viewer/mcviewer.c:380:73: note: each undeclared identifier is reported only once for each function it appears in

Let me know if you get Travis emails anyways and you don't need me to say this :-)

[andrew_b]

Replying to zaytsev:

Re. f462265, type is used inside of the condition.

Fixed.

Let me know if you get Travis emails anyways

No I don't.

[zaytsev]

Replying to andrew_b:

Let me know if you get Travis emails anyways

No I don't.

Normally, Travis will send notifications to the committer. At least this works in my case and I like them. If you don't get them, maybe you use a different email on Github as compared to the one you set in git. Also, maybe you need to login into Travis with your Github credentials once for it to pick it up: ​https://travis-ci.org/ ...

[andrew_b]

mc-3547-direntry.cremove-unused-variables.patch​: applied.

Branch rebased to resolve conflicts.

[andrew_b]

mc-3547-cleanup-args.c-reduce-function-scope-for-mcedit_arg_new.patch​: applied.
mc-3547-cleanup-mountlist.c-remove-unused-read_file_system_list-parameter.patch​: applied.
mc-3547-cleanup-setup.c-reduce-function-scope.patch​ added: applied.
mc-3547-cleanup-usermenu.c-use-const-edit_widget.patch​ added: applied.

[andrew_b]

Branch rebased to resolve conflicts.

[zaytsev-work]

Need to check whether this is also in (pulled from #1541), or else we could add this to the cleanup:

diff -Purp mc-4.6.2/vfs/extfs/ulha.in mc-4.6.2-rhs/vfs/extfs/ulha.in
--- mc-4.6.2/vfs/extfs/ulha.in    2009-02-01 19:30:21.000000000 +0000
+++ mc-4.6.2-rhs/vfs/extfs/ulha.in    2009-08-14 05:21:44.000000000 +0000
@@ -45,6 +45,12 @@ mc_lha_fs_list()
       $(NF) ~ /^\// { $(NF) =3D substr($NF,2) }
       # Print the line this way if there is no permission string
       $1 ~ /^\[.*\]/ {
+         # AR, PMA and CP/M LHARC lack date info
+         if ($6 =3D=3D "") {
+            $6 =3D $4
+            $5 =3D "00:00"
+            $4 =3D "01-01-1980"
+         }
          # Invent a generic permission
          $1 =3D ($NF ~ /\/$/) ? "drwxr-xr-x":"-rwxr--r--";
          # Print it
@@ -76,7 +82,7 @@ mc_lha_fs_list()
          # Well, that is the intent.  At the moment mc is translating them.
          split($2, id, "/");
          printf "%s 1 %-8d %-8d %-8d %s %s %s %s\n",
-                 $1, id[1], id[2], $3, $5, $6, $7, $8;
+                 $1, id[1], id[2], $3, $5, $6, $7, substr($0, 52);
          # Get the next line of the list
          next;
       }

[andrew_b]

Replying to zaytsev-work:

Need to check whether this is also in (pulled from #1541),

No. This ulha patch is not matched with lzip.

or else we could add this to the cleanup:

Yes.

...
       $(NF) ~ /^\// { $(NF) =3D substr($NF,2) }
...
+         if ($6 =3D=3D "") {
+            $6 =3D $4
+            $5 =3D "00:00"
+            $4 =3D "01-01-1980"
+         }
          # Invent a generic permission
          $1 =3D ($NF ~ /\/$/) ? "drwxr-xr-x":"-rwxr--r--";
...

BTW, patch is corrupted. I don't know why. =3D is quoted-printable representation of =.

[andrew_b]

Copy from ticket:3269#comment:14

Replying to and:

than

message (D_ERROR, MSG_ERROR, _("%d: %s"), (*mcerror)->code, (*mcerror)->message);

to

message (D_ERROR, MSG_ERROR, "%d: %s", (*mcerror)->code, _((*mcerror)->message));

as well, right?

Sorry for not chiming in earlier:

As a rule of thumb, anything but "%s" should be translated. This includes "%d: %s" and "%s (%d)". The French put a space before ":". The Japanese sometimes use some other characters for brackets.

[mooffie]

Trivial bug: WPanel doesn't report MSG_NOT_HANDLED for unhandled commands. I'm attaching a patch. It's a trivial fix.

Explanation:

Our widgets/dialogs usually have a WIDGET_execute_cmd() function constructed this way:

cb_ret_t WIDGET_execute_cmd (w, command)
{
    int res = MSG_HANDLED;

    switch (command) {
        case CK_Something: ...; break;
        case CK_Another:   ...; break;
        default:
          res = MSG_NOT_HANDLED;
          break;
    }

    return res;
}

But in the case of panel_execute_cmd() the "res = MSG_NOT_HANDLED" line was left out. The patch fixes this.

(I ​encountered the bug when I worked on the skin sampler.)

[andrew_b]

mc-3547-WPanel-should-report-MSG_NOT_HANDLED-for-unhandled-commands.patch​: applied.

[andrew_b]

Rebased to current master.
Initial commit: [2b6c45a21122e09b1c0329b2c0eb6c3c76ff8a91].
Please review.

TODO: update po/mc.pot and po/*.po after merge.

[andrew_b]

mc-3547-cleanup-Wlogical-not-parentheses-warning.patch​: applied
mc-3547-cleanup-Wconditional-uninitialized-warning.patch​: applied
mc-3547-cleanup-Wfloat-conversion-warning.patch​: applied.

@and: Andreas, please use a capital letter to begin a sentence in a patch comment. Thanks.

[zaytsev]

Re. review: I will try to have a look next weekend!

[andrew_b]

mc-3547-src-editor-edit.c-Cleanup-some-compiler-warnings.patch​: applied.
mc-3547-src-editor-editbuffer.c-Cleanup-some-Warnings.patch​: applied with some changes and additions.
mc-3547-src-editor-editwidget.c-Cleanup-some-compiler-warnin.patch​: applied.
mc-3547-src-editor-etags.c-Cleanup-some-compiler-warnings.patch​: another fix.
mc-3547-widget_options_t-Cleanup-Wassign-enum-warnings.patch​: applied with rename W_NONE -> W_DEFAULT with value 0.

[andrew_b]

Merged to master: [e8b68df26f6077772f8d3457440a5119ffd4c413].

git log --pretty=oneline 10a1c9d..e8b68df

[andrew_b]

That's all. Other cleanups welcome to next release.

[zaytsev]

download PO-translations from Transifex.net

done; [eae2a2542ea6b2b5afadcbec06f0f7ce7decb237]

download the hint translations from Transifex.net

done; [e52726963a04c2d1ad265d67b7c51431e322f83a]

create new NEWS wiki page for next version with empty template

done; wiki:NEWS-4.8.17

add content of current NEWS wiki page to the doc/NEWS file in git repo

done; [4e71328d7b7c5044386a598d9d9cf87507bdb8ba]

create new tag in git

done

new version in Trac

done

new milestone in Trac

done

create tar.(bz2|xz) package files

done

make checksums for archives

done

c37ea495df93f6d375a3ce59cdcf0f3a3690338526205347b03a316daebdaaf8  mc-4.8.16.tar.bz2
bbbcbe3097d3160f865d24aa38ff122f1c59752b5ef153ca4ade5ac0f82b7020  mc-4.8.16.tar.xz

upload source packages and checksums to the special upload area

done; ​​https://www.midnight-commander.org/nopaste/tarball/

developers should download tarballs, verify checksums, compile and install locally; if everything is ok, then developers vote for the ticket

done; please check it out!

[zaytsev]

(In #3553) I've checked it with the new release tarball that I've just generated and it seems to work fine now.

[zaytsev]

upload source packages and checksums to ​midnightcommander@…

done

run command: ssh midnightcommander@… '/home/midnightcommander/trigger-midnightcommander'

done

update Wiki start page with latest release number

done

write an announcement: list user visible changes (bugs and features)

done

create new ticket (type=task, component=adm) for the next release

done; #3607

close current milestone

done

close ticket for release

done