| # 00:08:20 | caveat- | quits |
| # 00:09:12 | caveat- | joins #crosstool-ng |
| # 00:21:39 | ragedragon | quits : Ping timeout: 250 seconds |
| # 04:44:39 | Net147 | quits : Quit: Quit |
| # 04:48:27 | Net147 | joins #crosstool-ng |
| # 05:56:08 | diorcety | joins #crosstool-ng |
| # 05:57:52 | lundmar | joins #crosstool-ng |
| # 06:12:08 | diorcety | quits : Read error: Connection reset by peer |
| # 06:31:31 | ragedragon | joins #crosstool-ng |
| # 06:43:18 | ragedragon | quits : Ping timeout: 276 seconds |
| # 07:20:17 | dummys | parts #crosstool-ng |
| # 07:27:00 | ghostbsdun | joins #crosstool-ng |
| # 10:01:47 | alan_o | quits : Ping timeout: 250 seconds |
| # 10:14:09 | alan_o | joins #crosstool-ng |
| # 16:01:33 | blueness | quits : Quit: blueness |
| # 16:12:13 | blueness | joins #crosstool-ng |
| # 16:14:07 | blueness | quits : Client Quit |
| # 16:14:25 | blueness | joins #crosstool-ng |
| # 16:41:23 | y_morin | joins #crosstool-ng |
| # 17:04:01 | ragedragon | joins #crosstool-ng |
| # 17:34:21 | ragedragon | quits : Ping timeout: 240 seconds |
| # 18:33:10 | ghostbsdun | quits : Ping timeout: 252 seconds |
| # 18:40:09 | blueness | quits : Quit: blueness |
| # 19:54:38 | aneyman__ | y_morin: 383? :) |
| # 19:55:11 | y_morin | aneyman__: Hello! Yes, later tonight. |
| # 19:55:14 | aneyman__ | review 1 commit a day, and we'll be done in less than a month :) |
| # 19:55:22 | y_morin | Meh... ;-) |
| # 20:14:34 | aneyman__ | y_morin: I've a question, btw |
| # 20:14:45 | aneyman__ | I have been perusing other pull requests |
| # 20:15:25 | y_morin | Yes? |
| # 20:15:25 | aneyman__ | I comment where I see some issues, but where I agree with the change - should I leave a note 'Reviewed-by: yours truly' or is it that only maintainers are supposed to do? |
| # 20:15:48 | y_morin | aneyman__: You can add your own reviewd-by tag, yes! |
| # 20:15:58 | aneyman__ | ok, thanks |
| # 20:16:18 | y_morin | aneyman__: It does really help maintainers of project when someone says "I've looked at that, and it looks good" |
| # 20:17:05 | aneyman__ | btw, I do think GH's review system is by far inferior to reviewboard (www.reviewboard.org) |
| # 20:17:30 | y_morin | Yes, github is very awfull for doing reviews. |
| # 20:17:37 | aneyman__ | I don't know if there are any services that employ reviewboard for GH repositories.... |
| # 20:17:44 | y_morin | largely prefers a mailing list coupled with patchwork... |
| # 20:18:05 | aneyman__ | the biggest feature lacking in GH is inability to track remaining issues |
| # 20:18:38 | aneyman__ | i.e. distinguish them from questions/comments/off-key discussions, and see their status (resolved/dropped/open) |
| # 20:18:42 | y_morin | aneyman__: What I find most annoying is the loss of history when a PR is rebased and repushed. |
| # 20:18:52 | aneyman__ | that was my 2nd note :) |
| # 20:18:54 | y_morin | aneyman__: Basically, what you say... |
| # 20:18:58 | y_morin | ;-) |
| # 20:19:14 | aneyman__ | but that at least can be worked around by not rebasing until a round of review is complete |
| # 20:19:19 | y_morin | gitlab does not fare much better.... |
| # 20:19:27 | aneyman__ | (provided the review takes some sane amount of time :P) |
| # 20:19:57 | aneyman__ | and the third is inability to quote a block of lines - GH only allows comments on a single line |
| # 20:20:22 | aneyman__ | whereas highlighting multiple lines provides clearer context for the comments in many cases |
| # 20:20:54 | y_morin | Yup. |
| # 20:22:11 | y_morin | aneyman__: I'm in the middle of a review in Buidlroot, I'll switch to #383 right after. |
| # 20:22:18 | y_morin | *Buildroot |
| # 20:56:20 | blueness | joins #crosstool-ng |
| # 21:00:05 | blueness | quits : Client Quit |
| # 21:02:15 | blueness | joins #crosstool-ng |
| # 21:06:27 | y_morin | aneyman__: OK, looking at #383 now.... |
| # 21:20:03 | blueness | quits : Quit: blueness |
| # 21:22:22 | ragedragon | joins #crosstool-ng |
| # 21:25:32 | y_morin | aneyman__: Did you re-push your branch recently? |
| # 21:26:03 | y_morin | Ah yes, git-pull gets new commits... |
| # 21:26:55 | y_morin | aneyman__: I guess your multilib-1 branch is for those commits I reviewed, right? |
| # 21:27:58 | y_morin | Ah! You split your series in two! Good! :-) |
| # 21:31:25 | y_morin | aneyman__: OK, restarting the numbering at 1 on your new multilib branch (considering the previous ten ones are OK): patch 1 is OK. |
| # 21:40:29 | ragedragon | quits : Ping timeout: 244 seconds |
| # 21:41:29 | aneyman__ | yeah, I split the first 10 commits into a new branch - once bhundven merges that, only the remaining commits should be displayed in #383 |
| # 21:41:44 | y_morin | aneyman__: Yes, that's sane! |
| # 21:42:01 | aneyman__ | yes, I repushed it - just noted a typo in sh.sh (in a patch that you haven't reviewed yet) |
| # 21:42:02 | y_morin | aneyman__: Patch 2: looks OK at first sight. |
| # 21:42:28 | y_morin | aneyman__: However, I'm a bit worried: |
| # 21:42:52 | aneyman__ | went unnoticed because we don't have a shX-*-uclibc sample where X != 4 |
| # 21:43:15 | y_morin | aneyman__: in scripts/build/cc/100-gcc.sh, the multilib case was previously guarded with an if-block, but now it seems unconditional |
| # 21:43:39 | y_morin | aneyman__: However, if there is no multilib reported by gcc, there is a warning that is being emitted. |
| # 21:43:52 | y_morin | So, I'm a bit uneasy with that... |
| # 21:44:42 | y_morin | Why do we now systematically check the multilib setup, even if CT_MULTILIB is not set? |
| # 21:44:59 | y_morin | Ah, yes, SuperH is a little beast I've never really played with... :-/ |
| # 21:45:08 | aneyman__ | which cset id are you reviewing? |
| # 21:46:11 | y_morin | aneyman__: e181893d: crosstool-NG.sh.in: Don't make lots of symlinks to lib folder |
| # 21:46:52 | aneyman__ | ok, I counted off by one :) |
| # 21:47:16 | aneyman__ | the difference here is that with this change, we're also counting the default multilib reported by gcc ('.') |
| # 21:47:43 | aneyman__ | AFAIR, let me double check |
| # 21:48:12 | y_morin | Oh, that makes sense. |
| # 21:48:14 | aneyman__ | yeah, see "tail -n +2" was changed into "tail -n +1" :) |
| # 21:48:38 | y_morin | Ah! |
| # 21:48:45 | y_morin | OK, that totally makes sense. |
| # 21:48:46 | aneyman__ | although tail -n +1 is kind of a no-op :) |
| # 21:49:10 | y_morin | is always confused with 1-based or 0-based counting... |
| # 21:49:30 | y_morin | tail counts as 1-based, right? So yes, the tail is superfluous... |
| # 21:49:37 | aneyman__ | but that is gone in the next commits |
| # 21:50:11 | aneyman__ | in the end, it is: multilibs=( $( "${cc}" -print-multi-lib ) ) |
| # 21:50:11 | aneyman__ | if [ ${#multilibs[@]} -ne 0 ]; then |
| # 21:50:41 | aneyman__ | tail is 1-based, correct |
| # 21:50:45 | y_morin | Yes, and there's at least '.' in the multilib list. |
| # 21:50:51 | y_morin | OK |
| # 21:50:52 | aneyman__ | exactly |
| # 21:51:26 | blueness | joins #crosstool-ng |
| # 21:51:33 | y_morin | 6025700 (glibc.sh: Matching CT_EndStep for multilib builds.) should probably have been folded into the corresponding commit, no? |
| # 21:53:02 | y_morin | Although I can't find the commit that adds an unbalanced DoStep.... :-/ |
| # 21:53:15 | aneyman__ | I left it separate because this bug (missing CT_EndStep) existed before this whole series |
| # 21:53:21 | aneyman__ | so it is a separate bugfix :) |
| # 21:53:25 | aneyman__ | see https://github.com/crosstool-ng/crosstool-ng/blob/master/scripts/build/libc/glibc.sh |
| # 21:56:53 | y_morin | Well, all I see is that we have DoStep on line 87 and 93 that are coupled with the EndStep on line 185; and a DoStep on line 107 coupled with a EndStep on line 181. So it looks like it is balanced? |
| # 21:59:31 | y_morin | Oh I found it! |
| # 22:00:08 | y_morin | aneyman__: d28a3dab (glibc.sh: Use --print-multi-os-directory) is removing a CT_EndStep. |
| # 22:02:02 | y_morin | Weird, somehow the commits listed in #383 are not the ones in your repository... :-/ |
| # 22:05:30 | aneyman__ | well because I picked up the multilib branch from mingwandroid's repository who (AFAIU) in turn picked it up from bhundven :) |
| # 22:05:53 | aneyman__ | so some of the commits were not authored by me |
| # 22:07:41 | aneyman__ | yeah, indeed DoStep/EndStep are matching in HEAD, so it was a flaw in mingwandroid's checkin |
| # 22:07:58 | aneyman__ | I didn't notice that and somehow assumed the HEAD was broken already |
| # 22:08:24 | aneyman__ | in that case, it can be folded, true, but is it worth another rebase (accompanied by loss of comments on GH)? :) |
| # 22:09:31 | aneyman__ | just tell me if you want me to fold it or keep it as-is |
| # 22:10:29 | y_morin | Well, if I were to apply the series (I'm not going to, bhundven is going ro) I could very well fld the two patches locally. |
| # 22:10:36 | y_morin | s/ro/to/ |
| # 22:10:45 | y_morin | s/fld/fold/ |
| # 22:10:54 | y_morin | ctngbot: Thank you! |
| # 22:10:54 | ctngbot | y_morin: no worries |
| # 22:12:47 | y_morin | s/fld/fold/; s/ro/to/ |
| # 22:12:57 | y_morin | Ah... Combos do not work... :-/ |
| # 22:13:25 | y_morin | Hmm... Because I spoke since then. |
| # 22:15:37 | aneyman__ | I don't know what GH interface for merging the PRs is - is it just some button on the website, or the regular fetch-merge-push dance? |
| # 22:16:57 | y_morin | aneyman__: Admins have a "Merge this Pull Request" button, yes. |
| # 22:17:22 | y_morin | However, I dislike it, because it does not allow simple tweaks like the one we;re discussing.... |
| # 22:18:57 | y_morin | One thing that Github sould do, is provide the comments as annotations to commits. So that, when I fetch your branch, I'd get all the existing annotations locally and I could review them, for example. |
| # 22:19:16 | y_morin | I mean, with git-notes |
| # 22:19:59 | aneyman__ | franly, I think git's ability to rewrite history is somewhat overused. It's okay when one polishes things before making them public - but once they are public, it shouldn't be folded/rewritten - rather, any fixes and review feedback should be on top of the changes |
| # 22:20:23 | aneyman__ | that way, one can push new changes without --force and GH won't lose the feedback |
| # 22:20:51 | aneyman__ | but as GIT book says, the policy varies by the projects, so all above is strictly IMO :) |
| # 22:29:38 | y_morin | aneyman__: OK, that's all for tonight. I've commented on another patch (the one that only build manuals/locales once): it was a bit complicated, and could be greatly simplified by building them on the *first* multilib instead of the last, no? ;-) |
| # 22:30:03 | y_morin | aneyman__: Yes, I know, that's not your original patch, you grabbed it. |
| # 22:33:26 | aneyman__ | I thought about it too |
| # 22:33:38 | aneyman__ | but decided to just not touch it |
| # 22:33:47 | aneyman__ | since it was done and (presumably) tested |
| # 22:34:03 | aneyman__ | and when I tried to enable building manuals, the build failed for me |
| # 22:34:13 | aneyman__ | so I wasn't able to test it myself |
| # 22:34:14 | blueness | quits : Quit: blueness |
| # 22:34:57 | aneyman__ | the failure was not due to multilib, though - texinfo choked on some TeX file produced by binutils, IIRC |
| # 22:36:00 | aneyman__ | you don't want me to add a 36th commit to the series to fix the build of manuals, do you? :) |
| # 22:37:05 | y_morin | Yes, I see. However, the manual should not build for you, even without this series. Does it build? |
| # 22:37:18 | y_morin | Anyway, time for some sleep! |
| # 22:37:19 | aneyman__ | no, it doesn't |
| # 22:37:24 | y_morin | Arg. |
| # 22:37:28 | y_morin | Anyway, time for some sleep! |
| # 22:37:30 | y_morin | Bye! |
| # 22:37:31 | aneyman__ | I actually just reinstalled my build host |
| # 22:37:38 | y_morin | quits : Quit: Nighty Night! |
| # 22:37:40 | aneyman__ | so I will try again on a newer ubuntu |
| # 22:45:45 | fenugrec | joins #crosstool-ng |
| # 22:46:27 | blueness | joins #crosstool-ng |
| # 23:39:55 | lundmar | quits : Quit: WeeChat 1.4 |