Ticket #3547 (closed task: fixed)

Opened 8 years ago

Last modified 8 years ago

Prepare for release mc-4.8.16

Reported by: zaytsev Owned by: zaytsev
Priority: major Milestone: 4.8.16
Component: adm Version: master
Keywords: Cc: egmont
Blocked By: Blocking:
Branch state: approved Votes for changeset: andrew_b

Description

See ReleaseGuidelines for more details.

Attachments

mc-3547-cleanup-conversion-warning-lock_c.patch (534 bytes) - added by and 8 years ago.
mc-3547-cleanup-conversion-warning-keybind_t.patch (18.8 KB) - added by and 8 years ago.
mc-3547-cleanup-conversion-warning-menubar_selected.patch (4.2 KB) - added by and 8 years ago.
mc-3547-cleanup-conversion-warning-gauge_c.patch (741 bytes) - added by and 8 years ago.
mc-3547-cleanup-conversion-warning-strutils_escape.patch (6.8 KB) - added by and 8 years ago.
mc-3547-cleanup-conversion-warning-mc_search_new.patch (13.2 KB) - added by and 8 years ago.
mc-3547-cleanup-conversion-warning-chown_advanced_but.patch (1.8 KB) - added by and 8 years ago.
mc-3547-cleanup-conversion-warning-chown_but.patch (1.2 KB) - added by and 8 years ago.
mc-3547-cleanup-conversion-warning-chmod_but.patch (3.2 KB) - added by and 8 years ago.
mc-3547-cleanup-conversion-warning-mode_t.patch (3.4 KB) - added by and 8 years ago.
mc-3547-remove-outdated-comment-about-refresh.patch (939 bytes) - added by mooffie 8 years ago.
mc-3547-fix-compiler-warning-at-search.c.patch (1.2 KB) - added by and 8 years ago.
mc-3547-listbox.c-remove-redundant-code.patch (875 bytes) - added by mooffie 8 years ago.
mc-3547-cleanup-add2hotlist_cmd-fix-Wformat-nonliteral.patch (1.3 KB) - added by and 8 years ago.
mc-3547-cons.saver.c-fix-Wformat-nonliteral-warning.patch (2.0 KB) - added by and 8 years ago.
mc-3547-cleanup-gcc-link-time-optimization-warnings.patch (3.5 KB) - added by and 8 years ago.
please double test fish section [read() replaced by lseek()]
mc-3547-treestore.c-cleanup-bit-fields-warnings.patch (2.4 KB) - added by and 8 years ago.
what more preferred? using bit fields and saving bytes or using gboolean for FALSE/TRUE state values
mc-3547-invalid-command-line-options-cause-segfault.patch (1.7 KB) - added by mooffie 8 years ago.
mc-3547-fish.c-cleanup-gcc-link-time-optimization-warning.patch (2.1 KB) - added by and 8 years ago.
tested with fish shell and large file transfer abort
mc-3547-robust-sizeof-usage-at-function-parameter-and-use-me.patch (30.3 KB) - added by and 8 years ago.
reduce possible future coding faults, binary checksum not changes but this patch
mc-3547-diffviewer.c-use-gboolean-at-WDiff-struct.patch (5.8 KB) - added by and 8 years ago.
mc-3547-cleanup-function-declare-at-USE_DIFF_VIEW.patch (745 bytes) - added by and 8 years ago.
mc-3547-remove-unused-const-INVALID_OFFSET.patch (1.3 KB) - added by and 8 years ago.
mc-3547-remove-unused-function-exec_shell.patch (1.2 KB) - added by and 8 years ago.
mc-3547-fish.c-cleanup-gcc-link-time-optimization-warning-v2.patch (2.1 KB) - added by and 8 years ago.
mc-3547-fish.c-cleanup-gcc-link-time-optimization-warning-v3.patch (2.2 KB) - added by and 8 years ago.
mc-3547-clarify-mc_args__debug_level-declare.patch (1020 bytes) - added by and 8 years ago.
mc-3547-clarify-edit_file-and-edit_stack_-declares.patch (2.3 KB) - added by and 8 years ago.
mc-3547-remove-unused-execute_with_vfs_arg-function.patch (11.9 KB) - added by and 8 years ago.
mc-3547-cleanup-internal-filemanager-layout-variables.patch (2.2 KB) - added by and 8 years ago.
mc-3547-cleanup-unneeded-mc_file-static-cast.patch (758 bytes) - added by and 8 years ago.
mc-3547-cleanup-unused-parent_call_string-function.patch (1.5 KB) - added by and 8 years ago.
mc-3547-clarify-macro-variables-for-use_internal_edit.patch (2.2 KB) - added by and 8 years ago.
mc-3547-cleanup-unused-option_edit_-_extreme-variables.patch (1.9 KB) - added by and 8 years ago.
mc-3547-Makefile-is-missing-subshell.h.patch (662 bytes) - added by mooffie 8 years ago.
(I didn't test this patch, sorry.)
mc-3547-direntry.cremove-unused-variables.patch (1.7 KB) - added by and 8 years ago.
mc-3547-cleanup-args.c-reduce-function-scope-for-mcedit_arg_new.patch (4.0 KB) - added by and 8 years ago.
mc-3547-cleanup-mountlist.c-remove-unused-read_file_system_list-parameter.patch (2.8 KB) - added by and 8 years ago.
mc-3547-cleanup-setup.c-reduce-function-scope.patch (8.3 KB) - added by and 8 years ago.
mc-3547-cleanup-usermenu.c-use-const-edit_widget.patch (3.6 KB) - added by and 8 years ago.
mc-3547-WPanel-should-report-MSG_NOT_HANDLED-for-unhandled-commands.patch (665 bytes) - added by mooffie 8 years ago.
mc-3547-cleanup-Wlogical-not-parentheses-warning.patch (1.5 KB) - added by and 8 years ago.
mc-3547-cleanup-Wconditional-uninitialized-warning.patch (1.8 KB) - added by and 8 years ago.
mc-3547-cleanup-Wfloat-conversion-warning.patch (2.3 KB) - added by and 8 years ago.
mc-3547-src-editor-edit.c-Cleanup-some-compiler-warnings.patch (3.4 KB) - added by and 8 years ago.
mc-3547-src-editor-editbuffer.c-Cleanup-some-Warnings.patch (1.9 KB) - added by and 8 years ago.
mc-3547-src-editor-editwidget.c-Cleanup-some-compiler-warnin.patch (1.6 KB) - added by and 8 years ago.
mc-3547-src-editor-etags.c-Cleanup-some-compiler-warnings.patch (1.3 KB) - added by and 8 years ago.
mc-3547-widget_options_t-Cleanup-Wassign-enum-warnings.patch (11.0 KB) - added by and 8 years ago.

Change History

comment:1 Changed 8 years ago by egmont

  • Cc egmont added

comment:2 Changed 8 years ago by slavazanko

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

comment:3 Changed 8 years ago by zaytsev-work

FIXFOR Ticket #3547: move subshell stuff into subdir.

src/subshell/common.c

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

comment:4 Changed 8 years ago by andrew_b

  • Blocking 3555 added

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

Changed 8 years ago by and

comment:5 Changed 8 years ago by and

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

comment:6 Changed 8 years ago by zaytsev

This ticket is the right one, thanks!

Changed 8 years ago by and

comment:7 follow-up: ↓ 14 Changed 8 years ago by 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.

Changed 8 years ago by and

comment:8 Changed 8 years ago by 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!

comment:9 Changed 8 years ago by zaytsev

  • Blocking 3556 added

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

comment:10 Changed 8 years ago by zaytsev

  • Blocking 3553 added

comment:11 Changed 8 years ago by zaytsev

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

Changed 8 years ago by mooffie

Changed 8 years ago by and

comment:12 Changed 8 years ago by andrew_b

  • Blocking 3561 added

comment:13 Changed 8 years ago by 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

comment:14 in reply to: ↑ 7 Changed 8 years ago by 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.

Changed 8 years ago by mooffie

comment:15 Changed 8 years ago by andrew_b

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

comment:16 follow-ups: ↓ 17 ↓ 18 Changed 8 years ago by 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?

comment:17 in reply to: ↑ 16 ; follow-up: ↓ 19 Changed 8 years ago by 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=*

comment:18 in reply to: ↑ 16 Changed 8 years ago by 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.

comment:19 in reply to: ↑ 17 ; follow-ups: ↓ 20 ↓ 21 Changed 8 years ago by 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.

comment:20 in reply to: ↑ 19 Changed 8 years ago by 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.

comment:21 in reply to: ↑ 19 Changed 8 years ago by 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.

comment:22 follow-up: ↓ 23 Changed 8 years ago by 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?

comment:23 in reply to: ↑ 22 Changed 8 years ago by 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.

comment:24 Changed 8 years ago by andrew_b

  • Blocking 3567 added

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

comment:25 Changed 8 years ago by 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

comment:26 Changed 8 years ago by and

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

Changed 8 years ago by and

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

Changed 8 years ago by and

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

comment:27 follow-up: ↓ 28 Changed 8 years ago by 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.

comment:28 in reply to: ↑ 27 Changed 8 years ago by 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.

comment:29 Changed 8 years ago by andrew_b

  • Blocking 3572 added

comment:30 follow-up: ↓ 32 Changed 8 years ago by 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?

comment:31 follow-up: ↓ 40 Changed 8 years ago by andrew_b

  • Branch state changed from no branch to on review

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.

comment:32 in reply to: ↑ 30 Changed 8 years ago by 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

Changed 8 years ago by and

tested with fish shell and large file transfer abort

comment:33 follow-ups: ↓ 34 ↓ 36 Changed 8 years ago by 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?

comment:34 in reply to: ↑ 33 Changed 8 years ago by 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.

Changed 8 years ago by and

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

Changed 8 years ago by and

Changed 8 years ago by and

comment:35 Changed 8 years ago by andrew_b

  • Blocking 3556 removed

comment:36 in reply to: ↑ 33 ; follow-up: ↓ 37 Changed 8 years ago by 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.

comment:37 in reply to: ↑ 36 ; follow-up: ↓ 38 Changed 8 years ago by 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) return you the size of pointer (4 or 8 bytes) not the size of allocated memory chunk.

Version 0, edited 8 years ago by andrew_b (next)

comment:38 in reply to: ↑ 37 ; follow-up: ↓ 42 Changed 8 years ago by 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.

comment:39 Changed 8 years ago by 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.

comment:40 in reply to: ↑ 31 Changed 8 years ago by andrew_b

  • Branch state changed from on review to merged

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

comment:41 Changed 8 years ago by andrew_b

  • Blocking 3555, 3561, 3567, 3572 removed
  • Branch state changed from merged to no branch

comment:42 in reply to: ↑ 38 Changed 8 years ago by 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.

comment:43 follow-up: ↓ 45 Changed 8 years ago by 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.

comment:44 Changed 8 years ago by 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.

comment:45 in reply to: ↑ 43 Changed 8 years ago by 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

comment:46 Changed 8 years ago by andrew_b

  • Blocking 3541 added

comment:47 Changed 8 years ago by andrew_b

New branch 3547_cleanup is created.
Initial changeset:f01fbce387e1fbbacd7a493d7bea9804195f094a

Last edited 8 years ago by andrew_b (previous) (diff)

comment:48 Changed 8 years ago by zaytsev

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

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

comment:49 Changed 8 years ago by zaytsev

  • Blocking 3379 added

comment:50 Changed 8 years ago by zaytsev

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

comment:51 Changed 8 years ago by andrew_b

  • Blocking 3541 removed

comment:52 Changed 8 years ago by 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.

comment:53 follow-up: ↓ 54 Changed 8 years ago by 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_'.

comment:54 in reply to: ↑ 53 Changed 8 years ago by 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.

comment:55 Changed 8 years ago by 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.

Changed 8 years ago by mooffie

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

comment:56 follow-up: ↓ 58 Changed 8 years ago by 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.)

comment:57 Changed 8 years ago by 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...

comment:58 in reply to: ↑ 56 Changed 8 years ago by andrew_b

Applied. Thanks!

comment:59 follow-up: ↓ 60 Changed 8 years ago by 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 :-)

comment:60 in reply to: ↑ 59 ; follow-up: ↓ 61 Changed 8 years ago by 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.

comment:61 in reply to: ↑ 60 Changed 8 years ago by 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/ ...

Changed 8 years ago by and

comment:62 Changed 8 years ago by andrew_b

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

Branch rebased to resolve conflicts.

comment:63 Changed 8 years ago by zaytsev

  • Blocking 3586 added

comment:64 Changed 8 years ago by 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.

comment:65 Changed 8 years ago by andrew_b

  • Blocking 3587 added

comment:66 Changed 8 years ago by andrew_b

Branch rebased to resolve conflicts.

comment:67 follow-up: ↓ 68 Changed 8 years ago by 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;
       }

comment:68 in reply to: ↑ 67 Changed 8 years ago by 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 =.

comment:69 Changed 8 years ago by 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.

comment:70 Changed 8 years ago by 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.)

comment:71 Changed 8 years ago by andrew_b

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

comment:72 Changed 8 years ago by andrew_b

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

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

comment:73 Changed 8 years ago by andrew_b

  • Branch state changed from no branch to on review

Changed 8 years ago by and

comment:74 Changed 8 years ago by 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.

comment:75 Changed 8 years ago by zaytsev

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

comment:76 Changed 8 years ago by 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.

comment:77 Changed 8 years ago by zaytsev

  • Votes for changeset set to zaytsev

comment:78 Changed 8 years ago by andrew_b

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

comment:79 Changed 8 years ago by andrew_b

  • Votes for changeset changed from zaytsev andrew_b to committed-master
  • Branch state changed from approved to merged

Merged to master: [e8b68df26f6077772f8d3457440a5119ffd4c413].

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

comment:80 Changed 8 years ago by andrew_b

  • Votes for changeset committed-master deleted
  • Branch state changed from merged to no branch

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

comment:81 Changed 8 years ago by andrew_b

  • Blocking 3379, 3553, 3586, 3587 removed

comment:82 Changed 8 years ago by andrew_b

  • Blocked By 3553 added

comment:83 Changed 8 years ago by 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!

comment:84 Changed 8 years ago by zaytsev

  • Owner set to zaytsev
  • Status changed from new to accepted
  • Branch state changed from no branch to on review

comment:85 Changed 8 years ago by zaytsev

  • Blocked By 3553 removed

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

comment:86 Changed 8 years ago by andrew_b

  • Votes for changeset set to andrew_b

comment:87 Changed 8 years ago by zaytsev

  • Status changed from accepted to testing
  • Resolution set to fixed
  • Branch state changed from on review to approved

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

comment:88 Changed 8 years ago by zaytsev

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