--- Log opened Tue Oct 29 00:00:48 2019
01:23 -!- belcher [~belcher@unaffiliated/belcher] has joined #rust-bitcoin
01:41 -!- jonatack [~jon@2a01:e35:8aba:8220:6627:dad:d967:649d] has quit [Ping timeout: 245 seconds]
02:06 < elichai2> BlueMatt: I don't have permissions in rust-bitcoin anyway
02:07 < elichai2> so me approving via github doesn't give you anything (unless you wanna give me permissions)
02:14 < elichai2> BlueMatt: FYI. I talked with Cory about playing with cross language LTO. so that might also be interesting
02:24 < elichai2> I see in your rusty branch in core you also have `libc`. who's depending on it?
02:37 -!- jonatack [~jon@213.152.162.15] has joined #rust-bitcoin
02:58 < stevenroose> BlueMatt: What was the reason for https://github.com/rust-bitcoin/rust-secp256k1/pull/177??
02:59 < stevenroose> Was it because compiling rust-secp was causing symbol collisions with Core? Or because you explicitly wanted to not compile libsecp twice??
02:59 < stevenroose> Because my secp256k1-sys PR solves the former.
02:59 < stevenroose> But it makes the latter very hard.
02:59 < stevenroose> I find it kinda sad that the PR was merged within 5 hrs and I didn't even see it pass by.
02:59 < stevenroose> Until now.
03:00 < stevenroose> elichai2: ^
03:03 < elichai2> stevenroose: yeah. you're right. we need to somehow wait/let people know about changes. altough it's not like there was a release. but maybe we need a better review process, idk.
03:03 < elichai2> anyway he didn't want to compile the secp here but to use core
03:03 < elichai2> *core's secp
03:14 < stevenroose> elichai2: are you sure? I mean I just did some on a Rust crate with C-exports that I used in a C codebase and because the C codebase also uses libsecp, it's just not possible to use rust-secp. Because all the symbols are defined twice. I think with Core he might have run into the same problem.
03:15 < elichai2> stevenroose: he compiles only the core's libsecp. not the one here (he uses rustc directly, not cargo)
03:15 < stevenroose> So one solution to that is to directly use the existing C symbols for rust-secp or to just isolate rust-secp more so that there is no interaction needed.
03:15 < elichai2> so the only symbols "twice" are the manual ones I wrote (which this feature disable)
03:16 < stevenroose> elichai2: hmm yeah using rustc directly makes this a lot easier, I guess
03:16 < elichai2> stevenroose: right. symbol mangling(like you did manually) will help with everything cargo based. altough it means you might have multiple versions of secp in your codebase
03:17 < stevenroose> So the only way we can still support this is to add a feature to my PR that disables the prefixing of the symbols in the Rust code.
03:19 < stevenroose> All won't get any prettier, that's for sure :D
03:19 < elichai2> if people agree that your solution is needed (I don't have a direct use case for me so hard for me to judge) than we can remove that new feature
03:19 < stevenroose> elichai2: one very direct use case is being able to make breaking changes to the rust-secp API.
03:20 < stevenroose> Right now we can't because it's impossible to have two rust-secp versions in your dependency tree.
03:21 < elichai2> hmmm
03:21 < elichai2> haven't thought of that
03:21 < stevenroose> Since that is kind of a rust-specific problem, I think it should have priority over all the C-integration ones. But of course we can/should try to support them if possible. Let me try to rebase.
03:21 < elichai2> so if you have 2 different major vesions of rust-secp it will collide
03:21 < stevenroose> elichai2: it's a known problem and the reason why rust-secp never updates :D
03:21 < stevenroose> elichai2: yeah the symbols collide because they both compile libsecp
03:21 < elichai2> now that I think about it I did have that problem with rocksdb in the past
03:21 < stevenroose> so my PR prefixes symbols with a version
03:22 < stevenroose> and with a rustsecp256k1_ tag so that it can also not collide with any other version-prefixed secp symbols that might occur in the wild
03:22 < elichai2> if `cc` and `bindgen` were the same crate I would've maybe write a feature that automatically does symbol mangling on compile time
03:22 < stevenroose> But I think I can add a feature no-prefix-symbols that can turn that off and then people can do whatever with the symbols.
03:23 < elichai2> stevenroose: with the way it works right now it would be quite hard
03:23 < stevenroose> elichai2: that's be cool
03:23 < elichai2> i'll try to ask alexchrichton about it
03:24 < elichai2> I had this idea for rust only too https://github.com/rust-lang/rfcs/issues/2785
03:30 < stevenroose> elichai2: would a feature external-symbols work for your use case? I could add that as a passe-partout for "dont_export_c_symbols" and a hypothecical "no_prefix_symbols"?
03:31 < elichai2> hmm so you'll have all the `extern "C"` declarations twice ones with prefix and another without? hmm
03:31 < stevenroose> that would make the rust code assume that it's building with symbols that are not the ones provided in the crate so that it would (1) use unprefixed symbols and (2) not export any symbols itself
03:32 < elichai2> I would wait for matt, as he already probably going to use LTO and cory is even looking into cross-language LTO than maybe your current PR gonna be enough for him
03:32 < elichai2> btw in your PR all my context_create/destory symbols that I implemented manually in rust are useless lol
03:33 < stevenroose> elichai2: no I think I can do a conditional tag like this     #[cfg_attr(not(feature = "external-symbols"), link_name = "rustsecp256k1_v0_1_0_ec_pubkey_tweak_add")]
03:33 < elichai2> (they were for bitcoinconsensus which does the opposite of what matt is doing. it uses rust-secp for core too)
03:33 < elichai2> ohhh I haven't remembered this attribute exists hmmm
03:33 < elichai2> let me test something
03:33 < stevenroose> cool
03:35 < stevenroose> btw, pms in IRC don't work well for me(should fix that), but you can always contact me on jabber: steven@konuro.net if we don't want to spam this channel as much :)
03:36 < elichai2> ha! https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=b566601be6ed6f27b24480de172986b2
03:36 < elichai2> awesome :)
03:43 < stevenroose> the link_name I already use and is tested in the secp-sys PR. But I could add a conditional to the feature so that we can support external symbols as well. I personally think it's a good solution.
03:51 < elichai2> yeah that sounds good I think
04:00 < elichai2> ok. I talked with `rust-bindgen` author. that will be quite simple to add there for autogenerated declarations
04:01 < elichai2> now I need to figure out if gcc/clang can support auto prefixing for symbols or we need to use objcopy for impl myself something related
04:07 < elichai2> ok. this is postponed to the weekend. otherwise i'll now spend the whole week researching into how to find and edit symbols in elf and exe (and whatever mac hell is doing) to implement a platform agnostic way for symbol prefixing in rust
04:16 < stevenroose> elichai2: do you have a setup where you can easily test building rust-secp with existing symbols?
04:16 < stevenroose> I have the code ready, but I think I will have to also add a special case to the build.rs
04:17 < stevenroose> I'm not sure if this would be sufficient `if cfg!(feature = "external-symbols") { return; }`
04:22 < elichai2> it wouldn't. one sec
04:23 < elichai2> you need to add  `println!("cargo:rustc-link-lib=static=secp256k1")`
04:23 < elichai2> and to test it copy `secp256k1.a` from a master rust-secp to yours and see it the tests pass
04:24 < elichai2> should be in `./target/debug/build/secp256k1-*/out/libsecp256k1.a`
04:27 < stevenroose> elichai2: Ehh, do you know if it's possible to enable a secp256k1 feature if you're just defining bitcoin as a dep? Can I just add secp256k1 as a dep as well? I can't add "secp256k1/external-symbols" to the bitcoin features..
04:27 < elichai2> Don't think I understand what you mean
04:28 < elichai2> Bitcoin has libsecp256k1.a as part of its linking process
04:30 < stevenroose> elichai2: more generally my Q is how can I enable a feature on a dependency of one of my dependencies?
04:30 < stevenroose> I'm depending on bitcoin, not on secp directly. But I want to enable some secp feature. I could add a proxy feature to bitcoin `external-secp-symbols = ["secp256k1/external-symbols"]`, but that's annoying
04:30 < elichai2> Like recovery?
04:31 < elichai2> Or ecdh?
04:31 < stevenroose> f.e.
04:31 < elichai2> Hmm recovery is enabled in bitcoin
04:31 < elichai2> Ecdh isn't
04:31 < stevenroose> but those features are different they "add" something, they don't "change" the build process.. but yeah imagine I want to enable ecdh
04:32 < elichai2> I don't think it's possible. If someone manually links he's responsible of making sure the correct symbols are available
04:32 < elichai2> So if he enables ecdh/recovery he also needs to make sure he's static lib has them enabled
04:35 < stevenroose> Could you try https://github.com/rust-bitcoin/rust-secp256k1/pull/169?
04:36 < stevenroose> I can't use the external-symbols in my own setup because I'm creating a static lib and I don't have a secp256k1.a artifact in my build process. I could go fetch one, though, but that'd defeat the purpose of my setup. Ah perhaps if I dylib it instead of staticlib
04:40 < stevenroose> Should we have a Travis test that tests the external-symbols feature? Or would an ack from someone actually having that situation do?
04:40 < stevenroose> BlueMatt: could you test if the external-symbols feature in #169 correctly replaces the dont_replace_c_symbols feature? https://github.com/rust-bitcoin/rust-secp256k1/pull/169
05:26 -!- jonatack [~jon@213.152.162.15] has quit [Ping timeout: 240 seconds]
08:04 -!- andytoshi [~apoelstra@wpsoftware.net] has joined #rust-bitcoin
08:04 -!- andytoshi [~apoelstra@wpsoftware.net] has quit [Changing host]
08:04 -!- andytoshi [~apoelstra@unaffiliated/andytoshi] has joined #rust-bitcoin
08:12 -!- andytoshi [~apoelstra@unaffiliated/andytoshi] has quit [Ping timeout: 250 seconds]
08:31 -!- instagibbs [~instagibb@pool-100-15-121-126.washdc.fios.verizon.net] has quit [Read error: Connection reset by peer]
08:33 -!- instagibbs [~instagibb@pool-100-15-121-126.washdc.fios.verizon.net] has joined #rust-bitcoin
08:50 -!- jonatack [~jon@2a01:e35:8aba:8220:6627:dad:d967:649d] has joined #rust-bitcoin
09:34 < stevenroose> request for review:
09:34 < stevenroose> https://github.com/rust-bitcoin/rust-bitcoin/pull/336
09:34 < stevenroose> https://github.com/rust-bitcoin/rust-bitcoin/pull/249
09:35 < stevenroose> and https://github.com/rust-bitcoin/rust-secp256k1/pull/169
10:10 -!- Dean_Guss [~dean@gateway/tor-sasl/deanguss] has joined #rust-bitcoin
11:57 -!- reallll [~belcher@unaffiliated/belcher] has joined #rust-bitcoin
11:58 < BlueMatt> elichai2: right now the full-rusty branch in core uses mio for socket handling for the p2p net layer, but we'll probably want to replace that, which would let us drop the libc requirement. The long-range-wireless-over-serial-adapter code has C++-ffi select() calls, instead of via libc
11:59 < BlueMatt> stevenroose: I dunno, we've had *lots* of discussion here and other repos about linking to core-built libsecp and linking this stuff in over the past week or two...
12:00 < BlueMatt> stevenroose: you're welcome to propose a *different* way to accomplish it, but that seems like the obvious one.
12:01 -!- belcher [~belcher@unaffiliated/belcher] has quit [Ping timeout: 276 seconds]
12:02 < BlueMatt> stevenroose: can you just *not* depend on secp256k1-sys at all if you build with external-symbols?
12:05 -!- reallll is now known as belcher
12:39 < wumpus> BlueMatt: for bitcoin core I don't see why a libc requirement would be a problem, it's not like bitcoin core will be compileable any time on platforms without a libc :)
12:39 < BlueMatt> wumpus: sorry, its the rust libc crte
12:39 < BlueMatt> which mostly just exposes libc, so I think thats fine
12:39 < wumpus> yes
12:39 < BlueMatt> but mio I want to go either read, or rip out
12:39 < BlueMatt> rip out may be easier, but I suppose I could also go audit it....
13:17 -!- Dean_Guss [~dean@gateway/tor-sasl/deanguss] has quit [Quit: Leaving]
13:37 < stevenroose> BlueMatt: which seems like the obvious one? about not using secp256k1-sys, I don't think so: -sys also defines the C-equivalent types in Rust.
--- Log closed Wed Oct 30 00:00:49 2019