{"action":"created","issue":{"url":"https://api.github.com/repos/capstone-rust/capstone-sys/issues/2","repository_url":"https://api.github.com/repos/capstone-rust/capstone-sys","labels_url":"https://api.github.com/repos/capstone-rust/capstone-sys/issues/2/labels{/name}","comments_url":"https://api.github.com/repos/capstone-rust/capstone-sys/issues/2/comments","events_url":"https://api.github.com/repos/capstone-rust/capstone-sys/issues/2/events","html_url":"https://github.com/capstone-rust/capstone-sys/issues/2","id":253524055,"number":2,"title":"I had to yank 0.3","user":{"login":"m4b","id":1920204,"avatar_url":"https://avatars2.githubusercontent.com/u/1920204?v=4","gravatar_id":"","url":"https://api.github.com/users/m4b","html_url":"https://github.com/m4b","followers_url":"https://api.github.com/users/m4b/followers","following_url":"https://api.github.com/users/m4b/following{/other_user}","gists_url":"https://api.github.com/users/m4b/gists{/gist_id}","starred_url":"https://api.github.com/users/m4b/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/m4b/subscriptions","organizations_url":"https://api.github.com/users/m4b/orgs","repos_url":"https://api.github.com/users/m4b/repos","events_url":"https://api.github.com/users/m4b/events{/privacy}","received_events_url":"https://api.github.com/users/m4b/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2017-08-29T04:10:09Z","updated_at":"2017-08-29T07:49:50Z","closed_at":null,"body":"This is primarily my fault for not doing an in depth review, and not wanting to maintain this very much, but imho, the crate is:\r\n\r\n1. a substantial downgrade from the previous version, 0.2\r\n2. has multiple bugs\r\n\r\nFor example, here are some major issues after trying to use it briefly:\r\n\r\n1. Using system binding balloons a fresh compile to 1-3 minutes - this is _completely_ unacceptable for essentially the rust version of a simple C `#include`. For comparison, compiling a fresh capstone on fcc8eac using system bindings was 1/2-2 _seconds_...\r\n2. None of the bindings are documented anymore!\r\n3. Related to 1, using the bundled bindings should never ever pull down bindgen, etc., and unrelated crates\r\n4. There's no reason to use `lazy_static` in the `build.rs` for a log file (the build isn't multithreaded); all this does is complicate the build and increase compile times.  Furthermore, there's no reason to have a build log file at all, as this is all printed directly in the output file in the respective deps directory. Even worse, passing `-vvv` doesn't print anything anymore; this just breaks user expectations.\r\n5. The feature flags aren't coherent - the `lib.rs` includes either the bundled or not-bundled depending on the feature flag, but `use_system_capstone` isn't reflected in this at all, nor is it depended on or dependent of the bundled flag\r\n6. There are non-existent feature flags in the `build.rs`, `\"build_src_cmake\"` (from the previous iterations); this indicates to me it was never tested.\r\n7. Bindgen dynamic output needs to be an optional feature flag; this should 100% not be the default\r\n\r\nThose are some of the major, show-stopping issues that forced me to yank.\r\n\r\nBesides this, I'm thinking that bundling capstone was a really bad move; this is old style C vendoring. There was nothing wrong with the git module before, it was easily updateable, and it didn't force _even a system binding user_ to download a 30MB crate.\r\n\r\nFurthermore, the argument that the user doesn't have an internet connection is:\r\n\r\n1. ok, then they should get their sys-admin or themselves to compile and install `libcapstone.so` and use the system bindings, or\r\n2. untenable; they just used cargo to either download this crate as a dep, or they got it from the internet somewhere.  Furthermore, the 4 dependencies and their 20 transitive dependencies need to be downloaded and compiled, so this is still untenable.\r\n\r\nI understand that git submodules are _sometimes_ undesirable, but I think this is a specific usecase where they are perfect precisely for what they're intended; and we don't force a 30MB download to someone who wants a 30k rust header file equivalent.\r\n\r\nThis brings me to the second point: using bindgen.  I'm not even convinced this is a motivating usecase for the capstone bindings.  As far as I know, the capstone ABI has been pretty rock-solid; I can't remember the last time it was changed, and I suspect it won't change anytime soon.  Using bindgen to generate bindings _only_ makes sense for the architecture header files which would have been prohitively difficult to write by hand; but could have been bindgen'd and then checked in quite easily, and the problem would have been solved.\r\n\r\nAgain, I should have downloaded, analyzed, and \"lived in\" the new version for a bit before merging and transferring this crate, but here we are.\r\n\r\nPlease don't take this in the wrong way; the current situation is just as much my fault as anyones, not having performed due process before the merge - but I hope we can rectify the situation, because as it stands now, I simply can't release these bindings as they stand.\r\n\r\nAt the end of the day, my decision to yank essentially came down to my answer to these questions:\r\n\r\n1. Would I be ok using this library and reading the documentation? - No\r\n2. Would I fork or write my own because it doesn't let me do what I want? - Yes\r\n\r\n```\r\n0.2 fresh compile, compiles capstone from source, after downloading from internet:\r\n    Finished dev [unoptimized + debuginfo] target(s) in 14.13 secs\r\n\r\n0.3 fresh compile, compiles bundled capstone from source\r\n    Finished dev [unoptimized + debuginfo] target(s) in 140.72 secs\r\n```"},"comment":{"url":"https://api.github.com/repos/capstone-rust/capstone-sys/issues/comments/325583378","html_url":"https://github.com/capstone-rust/capstone-sys/issues/2#issuecomment-325583378","issue_url":"https://api.github.com/repos/capstone-rust/capstone-sys/issues/2","id":325583378,"user":{"login":"tmfink","id":5265592,"avatar_url":"https://avatars1.githubusercontent.com/u/5265592?v=4","gravatar_id":"","url":"https://api.github.com/users/tmfink","html_url":"https://github.com/tmfink","followers_url":"https://api.github.com/users/tmfink/followers","following_url":"https://api.github.com/users/tmfink/following{/other_user}","gists_url":"https://api.github.com/users/tmfink/gists{/gist_id}","starred_url":"https://api.github.com/users/tmfink/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tmfink/subscriptions","organizations_url":"https://api.github.com/users/tmfink/orgs","repos_url":"https://api.github.com/users/tmfink/repos","events_url":"https://api.github.com/users/tmfink/events{/privacy}","received_events_url":"https://api.github.com/users/tmfink/received_events","type":"User","site_admin":false},"created_at":"2017-08-29T07:49:50Z","updated_at":"2017-08-29T07:49:50Z","body":"Thanks for the feedback @m4b; I don't want to make decisions in isolation.\r\nI think you make valid points--I'll see if I can make some improvements."}}
{"actor":{"display_login":"tmfink"}}