-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Prepare for release mc-4.8.19 #3693
Comments
|
|
|
|
|
|
|
|
|
|
|
|
|
|
Branch: 3693_cleanup.
mc-3641-cleanup-Wformat-signedness-common_c.patch: applied. |
|
|
3693-Hex-patterns-fix-manual-page.patch: applied. |
FWIW: I've read all the commits in this branch (except "Clarify startup" and "fix location of right panel", for lack of time) and they seem alright.
(As for ditching negative numbers in hex patterns (hex.c): that's a blessing (at least as long as nobody volunteers to implement reading words/dwords/etc). Besides, reading them with "%x" meant that if you typed "-10" you got -16.) |
re: "Clarify usage of assert":
There are two places that use g_assert() (and 3 places that use g_assert_*()).
The purpose of AC_HEADER_ASSERT is to make --disable-assert define the NDEBUG constant, which disables assert(). But to disable g_assert() one has to define the G_DISABLE_ASSERT constant.
So, it seems to me, we should choose either assert() or g_assert(), not both.
It's easier to just rename the two g_assert() we have to assert().
(One may argue that there are a few g_assert_*() functions we may covet (like g_assert_cmpint), and that for the sake of stylistic uniformity we should therefore prefer g_assert() to assert(). But these g_assert_*() bunch can't be disabled, so we may treat them as a category of their own.)
Not that I care too much about any of this. It's just that I'm about to propose a patch in which I was planning to use g_assert() / assert() and I don't want to get some flak ;-) |
Replying to mooffie:
Indeed.
I prefer g_assert() and I've added the commit. |
@andrew_b: just a nitpick, the plural of assert is not "Asserts:", but "Assertions:". |
Here too: "Define to disable asserts" -> "Define to disable assertions". |
Rebased to current master: [e036c1fb12757dc9d258862736054c33313a51d5]. |
|
This documentation is needed in order to understand the bug(s) in #3720. |
3693-mc_search_run-document-the-return-value.patch: applied.
Rebased to current master: [645e88d209a520e6c83eeced710b9376f878c8f8]. |
|
|
Andrew, are you fine with committing the patch above? |
Replying to zaytsev:
|
Ah, ok thanks, sorry, I've got completely confused in the end. |
It seems that all is calm, so I'm planning to pull in latest translations and make the release this weekend, probably on Sunday, unless there are objections to this plan. |
Sounds great! |
done
done
done; nothing new
skip
done; NEWS-4.8.20
done
done
done
done
done
done
done
http://midnight-commander.org/nopaste/tarball/
please! |
I hardly ever remember to run "make check", but when I do:
The corresponding log files contain this:
The tests succeed for me with the 4.8.18 tarball. |
Unfortunately, you are right, good catch! I have reproduced the problem on my Fedora VM, but I have no idea what that is. These tests are failing permanently on Travis, which I why I assumed it has something to do with Travis environment proper. I've started a bisection run between 4.8.19 and 4.8.18 to narrow down the culprit, but it's progressing very slowly. |
I'm also running git bisect, it's almost finished :) You can stop yours.
Just wondering, is "make check" not executed in the continuous build, and as part of the release process? If so, at least the release process documentation should be extended with this entry. |
[b91ab44] is the first bad commit |
I've got ~7 commits to go, so I guess I'll leave it running anyways just to see if I can match your results ;-)
It definitively is. The problem is, some tests fail on Travis for mysterious reasons and Andrew couldn't reproduce it, and others are failing for reasons that I understood, but was not able to find time to fix so far. Therefore, I had to make Travis builds only fail when the build itself fails, not upon test failures, which obviously make real problems easy to overlook... :-( but it's better than not running them at all, at least I sometimes inspect the logs visually.
Actually, implicitly it is, I just skipped it because it all takes so long on my underpowered laptop :-/ but you are right, this is not good. I have added it to the guidelines. |
Replying to zaytsev:
Mandatory link: #2252 :P
Thanks! |
Confirm [b91ab44] ! Good thing the commit is so small, that's why we want them to be like this --- upon problems bisect points you directly to where the trouble is. But I'm confused as to why marking noreturn function as such makes the test fail. Need to have a closer look later :-/ |
Well, the way I do bisects is as follows, and this is already a shortcut, so in this particular case #2252 is irrelevant
|
Ha, maybe I should have read the comment why this function was introduced in the first place:
So apparently the warning should have been suppressed instead! |
Replying to zaytsev:
I didn't know about this possibility, I'm always doing it "manually".
But... apparently this construct cannot differentiate between failed tests (where you'd type "git bisect bad" and failed builds ("git bisect skip").
Anyway, it's always good to learn new practices :) |
w00t!!!11 you are a man of great patience... I always start bisects in the background if results are easily verifiable by script and do other stuff while they are running.
This is just because I know that there will be no failed builds in master, but otherwise I would add make || exit 125 to tell git to skip the commit if the build fails.
Okay, as to the commit, I propose a revert + a follow up commit to clarify that the comment function should not be decorated.
Then I will make a new tag and rebuild the tarballs. I haven't pushed the tag out yet. |
Re-built and re-uploaded the tarballs. |
And again, today is definitively not my day :-( |
What was wrong with the comment on the same line? |
Replying to egmont:
It confused GNU Indent and as it tried to reformat the thing, it was breaking the formatting completely, but somehow I didn't notice it until it was too late :-/ Oh well, the bottom line, it likes it when it's on a separate line and it seems that all is good now. |
|
|
done
done
done
done
in a moment
done; #3780
done |
Hmmmm... apparently forgot to close this. |
Important
This issue was migrated from Trac:
zaytsev
(@zyv)egmont
(@egmontkob)Note
Original attachments:
andrew_b
(@aborodin) onSep 26, 2016 at 7:44 UTC
andrew_b
(@aborodin) onSep 26, 2016 at 7:44 UTC
andrew_b
(@aborodin) onSep 26, 2016 at 7:44 UTC
andrew_b
(@aborodin) onSep 26, 2016 at 7:44 UTC
andrew_b
(@aborodin) onSep 26, 2016 at 7:44 UTC
andrew_b
(@aborodin) onSep 26, 2016 at 7:45 UTC
andrew_b
(@aborodin) onSep 26, 2016 at 7:45 UTC
andrew_b
(@aborodin) onSep 26, 2016 at 7:45 UTC
andrew_b
(@aborodin) onSep 26, 2016 at 7:45 UTC
andrew_b
(@aborodin) onSep 26, 2016 at 7:45 UTC
andrew_b
(@aborodin) onSep 26, 2016 at 7:46 UTC
andrew_b
(@aborodin) onSep 26, 2016 at 7:46 UTC
andrew_b
(@aborodin) onSep 26, 2016 at 7:46 UTC
andrew_b
(@aborodin) onSep 26, 2016 at 7:46 UTC
mooffie
(@mooffie) onNov 1, 2016 at 13:08 UTC
mooffie
(@mooffie) onNov 12, 2016 at 21:14 UTC
mooffie
(@mooffie) onDec 1, 2016 at 14:40 UTC
mooffie
(@mooffie) onDec 5, 2016 at 0:22 UTC
and
onDec 6, 2016 at 20:54 UTC
and
onDec 6, 2016 at 20:54 UTC
and
onDec 6, 2016 at 20:54 UTC
and
onDec 6, 2016 at 20:54 UTC
and
onDec 6, 2016 at 20:55 UTC
and
onDec 6, 2016 at 20:55 UTC
and
onDec 6, 2016 at 20:55 UTC
and
onDec 6, 2016 at 20:55 UTC
and
onDec 6, 2016 at 20:55 UTC
and
onDec 6, 2016 at 20:55 UTC
and
onDec 6, 2016 at 20:55 UTC
and
onDec 6, 2016 at 20:56 UTC
The text was updated successfully, but these errors were encountered: