< wumpus>
jonasschnelli: yes, I'd also say remove support for qt4. But apparently luke-jr and BlueMatt are still using that
< jonasschnelli>
wumpus: Yes. I just re-read the Qt4 EOL issue...
< jonasschnelli>
Maybe keep it for now... at least we should fix the compiling issue.
< wumpus>
it's kind of annoying, but that's what you get with 'big step' API changes, the old version will virtually stay around forever
< wumpus>
same with python2/3
< jonasschnelli>
Yeah.
< wumpus>
though qt4->qt5 is nowhere as much work as py2->3
< jonasschnelli>
Adding qt4 compilation over travis seems to be not worth it... I guess?
< wumpus>
I'd prefer not to
< jonasschnelli>
Okay. Lets fix then the compilation issue whenever someone reports them.
< luke-jr>
wumpus: and we don't need to support Py2 because Py3 is everywhere and works as well
< wumpus>
I'd prefer some timeframe for dropping qt4, but should be announced upfront
< luke-jr>
jonasschnelli: I already added Qt4 to Travis a while ago, and was under the impression we had a daily (not every PR) running it
< luke-jr>
it's funny how Qt5 is more compatible with GTK2 than it is with Qt4
< wumpus>
ironically qt3->qt4 was a bigger step, and people ported software over quicker
< luke-jr>
they did? O.o
< luke-jr>
IIRC KDE 4 took a looong time
< luke-jr>
and even today, KMail isn't up to par with the Qt3 version
< wumpus>
how I remember things they did, almost every project saw the need of supporting qt4 immediately, and it was very hard to support both 3 and 4 so 3 was pretty quickly dropped
< wumpus>
qt5 on the other hand, maybe because it's releatiively easy to suport both 4 and 5, takes longer
< wumpus>
but it's just anecdotal I don't have numbers to back it up
< wumpus>
qt5 does have some nice new features the thing is we don't really need them, at least not for the essential features
< luke-jr>
you mean dropping the old vs supporting the new, then?
< wumpus>
I gues
< luke-jr>
IMO old should only be dropped when it becomes a burden; so Qt4->Qt5 being minimal delays that because the burden is somewhat low
< wumpus>
yes I mean qt3 was dead sooner than qt4
< wumpus>
I'm fine with supporting qt4 as long as some of us compile with that and use it
< wumpus>
if not it means it goes largely untested
< wumpus>
in any case i'm not worried about a "doesn't compile with qt4" issue once in half a year which usually gets promptly fixed
< wumpus>
I'm much more worried about the wshadow diff noise
< wumpus>
that's more work to support than qt4
< jonasschnelli>
wumpus: wshadow: you mean because of your statement: "There's a large chance that one of the 'fixes' for WShadow, which seem to trail any larger change in the source code, introduce a bug."?
< bambum>
last time I was told core dont want to decide about what people want to run, but some commits being closed and locked after 4 minutes about block site increase should be at least commented why they are closed and locked https://github.com/bitcoin/bitcoin/pull/10092
< bitcoin-git>
bitcoin/master 4df76e2 Andrew Chow: Ensure an item exists on the rpcconsole stack before adding...
< bitcoin-git>
bitcoin/master 0ddea44 Jonas Schnelli: Merge #10060: [Qt] Ensure an item exists on the rpcconsole stack before adding...
< bitcoin-git>
[bitcoin] jonasschnelli closed pull request #10060: [Qt] Ensure an item exists on the rpcconsole stack before adding (master...fix-rpcconsole-empty-stack) https://github.com/bitcoin/bitcoin/pull/10060
< gmaxwell>
bambum: presumably because it was broken garbage-- would instantly break everything, didn't even pass the tests-- and submitted by someone who has made nasty comments in the past. No one is going to waste their time on trolling.
< wumpus>
bambum: because it is trolling
< wumpus>
he's not exactly the first to open a pull like that
< wumpus>
it's not funny, it's not constructive
< wumpus>
the only sensible reply is to block it and ignore it and continue on as normal
< bambum>
ok, but for better transparency it should be commented, just a copy past would be enough "trolling" is not specified. For someone like me it hard to know, I thought also it might not fit into some rules, but a clear comment why it is closed would be good
< wumpus>
also this is not about 'deciding about what people want to run' but concerns our project
< wumpus>
attacking our project with random PRs is only going to get you banned
< wumpus>
that's the end of this discussion. Please try to keep drama out of this channel.
< bambum>
there was nowhere drama in my questions, it was a fair question, you are over interpreting it
< bambum>
I wish more transparency
< jonasschnelli>
bambum: please...
< wumpus>
we are not obliged to give any motivation for closing issues, or giving you any extra information. Most of the people involved here are volunteers and their time they can spend on this project is very limited and better directed to useful purposes
< jonasschnelli>
You need to understand how tiresome such PRs are... it's like trowing a stones into gears.
< jonasschnelli>
s/a//
< wumpus>
jonasschnelli: yes, it's just tiring, drags on and on and on
< wumpus>
at some point you get annoyed and just close things
< bambum>
@jonasschnelli yes but how can I recognize it as someone not being involved in techs ?
< gmaxwell>
Any any comment left is just trolled against.
< wumpus>
bambum: again, this channel and github is for doing development
< bambum>
so you saying you can recognize its trolling just by seeing the code ?
< bambum>
@wumpus you dont understand the point
< jonasschnelli>
bambum: It's like changing the default port number from 80 to 13735 in apache... It's not serious.. everyone at bitcoin github level must understand this...
< wumpus>
bambum: maybe not, but you're starting to annoy me
< bambum>
I am talking about transparency, everyone should now what is happening
< gmaxwell>
bambum: sure, like the fact that it didn't even pass the selftests. it wasn't an actual proposal for anything.
< bambum>
know
< jonasschnelli>
bambum: It's clear what happend. Somebody started to throw a stone. It got rejected. Fullstop.
< wumpus>
bambum: this is the second time in a few days that you're monopolizing this channel with discussion about non-development things
< jonasschnelli>
Yes. Let's stop this here.
< jonasschnelli>
Move this on to #bitcoin if you like
< wumpus>
please mind the topic of this channel "This is the channel for developing Bitcoin Core. Feel free to watch, but please take commentary and usage questions to #bitcoin"
< bambum>
wumpus this was something what happend on the github website, last time i was here in the channel I was told that core dont want to influent peoples what do to, so I had a question about
< wumpus>
we don't influence 'peoples what to do' but we do influence our own project
< bambum>
I appreciate having the option to talk about it
< bambum>
and the only think i am requesting is more transparency
< gmaxwell>
bambum: request heard. now please stop repeating it.
< gmaxwell>
(and while you're at it, go look at the post history of the person making that PR, you'll find it quite informative)
< bambum>
jonasschnelli yes for me it was not so easy to understand it
< bambum>
I just had the idea that his code don´t fits into some rules
< bambum>
but needs to be commented imo, especially nowatimes
< gmaxwell>
K.
< wumpus>
jonasschnelli: at least github deleted the fake jonasschneli account
< jonasschnelli>
wumpus: heh. Yes. I reported it and ~48h later they deleted it.
< jonasschnelli>
Not superfast but still okay.
< bitcoin-git>
[bitcoin] jonasschnelli opened pull request #10093: [Qt] Don't add arguments of sensitive command to console window (master...2017/03/qt_console) https://github.com/bitcoin/bitcoin/pull/10093
< bambum>
@gmaxwell thanks for commenting the commit, imo it would be totally enough to list like "locked for -" or "banned for -" 1. Submitting broken code 2. Not passing self-tests 3. Breaking network .. next time. Even a bot can do this, if devs prefer to not comment themself the lock or the bann. Would save up some energy.
< wumpus>
seems unrelated to that pull though, I don't get why it starts failing there
< wumpus>
or why it works now
< jonasschnelli>
Yes. I wonder why it works now...
< bitcoin-git>
[bitcoin] laanwj opened pull request #10095: refactor: Move GetDifficulty out of `rpc/server.h` (master...2017_03_getdifficulty_header) https://github.com/bitcoin/bitcoin/pull/10095
< wumpus>
jonasschnelli: we didn't use to use boost test framework in the qt tests
< jonasschnelli>
But your PR doesn't change that? Or does it?
< wumpus>
no, it doesn't do anything with that
< wumpus>
makes sense to PR that separately
< wumpus>
uhm, we're still not using boost::test in the qt tests, I don't understand why we'd need that library
< jonasschnelli>
Okay. I'll PR it.
< wumpus>
well let's try to find out why it's needed first, I'm not sure anymore, I thought we had a good reason, but there's no reference to boost test in the qt tests
< wumpus>
so that change should not be necessary :/
< jonasschnelli>
It's seems to be for the logging....
< wumpus>
the only three occurences of 'boost' or 'BOOST' in src/qt/test are: #include <boost/filesystem.hpp> boost::filesystem::remove_all boost::signals2::scoped_connection
< wumpus>
boost is not used for logging nor evaluating test cases
< * wumpus>
confused
< wumpus>
argh
< wumpus>
wallettests.cpp includes test/test_bitcoin.h
< wumpus>
maybe that indirectly imports some boost test stuff?
< wumpus>
ideally the qt tests would test with a mocked wallet model instead of importing all the core stuff
< jonasschnelli>
hmm. Yes. That probably the issue.
< wumpus>
oh it even links against test_bitcoin.cpp
< jonasschnelli>
wumpus: but even after I remove the #include "test/test_bitcoin.h" (and remove the test code), I still get the linker issue
< wumpus>
that one certainly uses boost::test
< jonasschnelli>
ah... thats it!
< jonasschnelli>
I don't think this can be fix easily.
< wumpus>
bleh, the test utils should not themselves rely on any test framework
< wumpus>
e.g. testutil.cpp is fine
< wumpus>
I don't think so either, I think it was mistake to make the qt and normal tests interdependent like this
< wumpus>
anyhow no big deal to make qt tests depend on boost::unittest
< wumpus>
at least we know why, now
< jonasschnelli>
But why does this work in current master?!
< wumpus>
maybe we should go all the way and make the qt tests a boost test runner as well... but fixing it for now is easy just add the lib
< wumpus>
I don't know
< jonasschnelli>
wumpus: Yes. Let's fix the missing lib add, then let's see if ryanofsky is up for a clean split
< wumpus>
well it needs to be one of either: either the qt tests go fully boost::unit_test, or the shared code between the two test suites should be independent on the test framework
< wumpus>
either is fine with me, but using a few functions from boost::unit_test through linking in test_bitcoin.cpp *without* using the framework is asking for trouble
< wumpus>
e.g. what happens if you do BOOST_ASSERT while not in a boost unit test
< jonasschnelli>
Indeed
< wumpus>
hm! but seems he took that into account
< wumpus>
I don't see references to boost_test in test_bitcoin.cpp
< wumpus>
it's no longer the test main file
< wumpus>
what, I don't get it, I remember I added some BOOST_REQUIRE to TestingSetup at some point
< bitcoin-git>
[bitcoin] jnewbery opened pull request #10096: Check that all test scripts in test/functional are being run (master...check_all_tests_run) https://github.com/bitcoin/bitcoin/pull/10096
< BlueMatt>
jonasschnelli: as noteed in the issue qt4 -> qt5 introduces regressions
< BlueMatt>
we need to fix those before we can turn it off
< BlueMatt>
major regressions that break peoples' ability to use Bitcoin-Qt, that is
< bitcoin-git>
[bitcoin] jnewbery opened pull request #10097: Move zmq test skipping logic into individual test case. (master...zmq_optional) https://github.com/bitcoin/bitcoin/pull/10097
< bitcoin-git>
[bitcoin] kallewoof closed pull request #10062: [net] Clean up the CNode class in net.h (master...20170322-cleanup-net) https://github.com/bitcoin/bitcoin/pull/10062
< bitcoin-git>
[bitcoin] JeremyRubin opened pull request #10099: Speedup & Slightly Improve Unit Tests for Checkqueue (master...speedup-checkqueue-tests) https://github.com/bitcoin/bitcoin/pull/10099
< BlueMatt>
jonasschnelli: ping
< bitcoin-git>
[bitcoin] RHavar opened pull request #10100: Make ApproximateBestSubset optimize for amount of inputs (master...coinselection) https://github.com/bitcoin/bitcoin/pull/10100
< bitcoin-git>
[bitcoin] ryanofsky opened pull request #10102: bitcoin-qt: spawn bitcoind and communicate over pipe (Experimental, WIP) (master...pr/ipc) https://github.com/bitcoin/bitcoin/pull/10102
< cfields>
ryanofsky: cool!
< achow101>
does anyone know if the way that Core 0.14 broadcasts messages has changed from the way the 0.13.2 did?
< sipa>
can you be more specific?
< achow101>
I guess more specific would be how network packets are being sent
< sipa>
there have been significant changes to that, yes
< sipa>
not sure whether those should be observable, though
< achow101>
I think I mentioned this several months ago when I first started looking into this issue. On Armory, I have noticed that it was receiving and processing message headers separately from the message payloads
< achow101>
it would appear that this behavior is caused by something in 0.14.0 since people have only been reporting the issue since 0.14.0's release
< achow101>
I only noticed because I always run a build of the master branch.
< sipa>
define 'separate' ?
< achow101>
it would interpret the message header as a message, and then interpret the message payload as a completely new message
< sipa>
TCP does not have messages
< sipa>
it is a byte stream
< achow101>
message being the bitcoin p2p messages
< cfields>
achow101: yes, that's possible now
< sipa>
that makes no sense... a p2p message is defined as a header + payload
< sipa>
you're asking whether a message can consist of a message and a message
< phantomcircuit>
sipa, im guessing they screwed up and are assuming the entire message will be received in a single recv()
< cfields>
sec for some grepping so i can use better terms
< sipa>
phantomcircuit: yes, that's my assumption too, but that would have always been a bug
< achow101>
phantomcircuit: well we never saw this issue before 0.14.0
< phantomcircuit>
sipa, yeah but wouldn't have been easily triggered until recently
< BlueMatt>
phantomcircuit: in that case you'd also see it for big messages
< BlueMatt>
(>~1k)
< sipa>
achow101: yes, it's now done as separate send calls
< cfields>
TCP_NODELAY + separate sends
< phantomcircuit>
BlueMatt, not if they have a huge recv buffer and are on localhost always
< phantomcircuit>
which they are
< achow101>
we fixed the issue on our end, but I just wanted to figure out why that happened since it only appeared when people used 0.14.0
< cfields>
make it likely that you'll receive the header in the first chunk
< BlueMatt>
phantomcircuit: you still usually see it for things >>1k
< BlueMatt>
eg 1MB you'd def see it
< phantomcircuit>
uh
< phantomcircuit>
hmm let me see
< phantomcircuit>
i dont think that's right
< BlueMatt>
localhost can be surprisingly slow somteimts
< BlueMatt>
sometimes
< sipa>
achow101: so, yes, there has been a change where header and payload are now sent through separate kernel calls, which makes it much more likely you'll see them in separate recv() calls
< achow101>
ok. thanks
< achow101>
we fixed the problem, I just wanted to know why it was a problem in the first place
< sipa>
it should always have been possible to see this; TCP does not guarantee that message boundares are preserved
< gmaxwell>
it's easy in tcp applications to write bugs where you assume your reads will always contain a complete message... when the remote end used a single write (or nagle merged them)... then something changes and the reads are short.
< sipa>
but on localhost i guess it would be unlikely
< achow101>
well I don't know how it was originally implemented in Armory nor how it was fixed. I just know that one day it magically started giving me errors for something that had never happened in a previous version of core
< achow101>
anyways, thanks for the info
< cfields>
well also if you're receiving a few messages in one read, it'd be very likely that a header would span 2 chunks
< cfields>
so seems strange that that would've ever worked :)
< phantomcircuit>
BlueMatt, why do i get two sendcmpt messages for every connection?
< BlueMatt>
phantomcircuit: thats how the version negotiation works