TriggerMail / rules_pyz

Bazel Python rules that package everything in an executable zip
Apache License 2.0
29 stars 19 forks source link

rm simplepack, exec_template.sh: Now unused #10

Closed evanj closed 6 years ago

evanj commented 6 years ago

simplepack was replaced by linkzip.py, which removes a dependency on Go.

joshclimacell commented 6 years ago

Whoa awesome! I guess I learned go for nothing now though, haha.

evanj commented 6 years ago

Ha. I personally like Go more than Python, but it is annoying for rules for Python to depend on Go, particularly now that the performance of zipping these rules is not critical, since it probably doesn't need to happen as often (e.g. it isn't needed for tests).

BTW: I was going to try to email you or something: I almost certainly have broken your changes; sorry: I do plan on fixing them back up shortly I hope! This version is much faster to test stuff on our code base, so hopefully the improvements are worth it, and now we can figure out how to correctly support both Python2 and Python3!

joshclimacell commented 6 years ago

Great, thanks Evan! No prob! Would now be a good time for me to experiment with your new changes, or are things still in flux? Hopefully at some point we can start keeping everything in sync.

evanj commented 6 years ago

I still need to merge and commit my pip_generate changes; let me try to get that merged today. Once that lands, I'd love feedback! In particular: This version should be faster to build; in particular it should be much faster to rebuild a target after you make a change. I'd love to know your experience.

I'd also like to add Python3 support to the CircleCI configuration so I don't break it inadvertently, so I'd love to know how you are using it.

joshburkart commented 6 years ago

Sounds great! Looking forward to it.

On Mon, Jun 4, 2018 at 9:46 AM Evan Jones notifications@github.com wrote:

I still need to merge and commit my pip_generate changes; let me try to get that merged today. Once that lands, I'd love feedback! In particular: This version should be faster to build; in particular it should be much faster to rebuild a target after you make a change. I'd love to know your experience.

I'd also like to add Python3 support to the CircleCI configuration so I don't break it inadvertently, so I'd love to know how you are using it.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/TriggerMail/rules_pyz/pull/10#issuecomment-394360298, or mute the thread https://github.com/notifications/unsubscribe-auth/ADtUNRfatDjvkBYUoIf3EeWwHguA0AAHks5t5TpBgaJpZM4UX09h .

joshburkart commented 6 years ago

Also, here's a list of the changes I've made in my fork, in case it informs your work: https://github.com/climacell/rules_pyz/commits/master

I think this is the high-level summary:

  1. Handle PyPI packages that only provide METADATA files (and not metadata.json IIRC)
  2. Handle PyPI packages that place all their code in -.data/purelib
  3. Handle dots in PyPI package names
  4. Handle cases where there are two suitable wheels for a platform
  5. Provide option to delete locally stored wheels that are no longer used

On Mon, Jun 4, 2018 at 10:17 AM Josh Burkart jburkart@gmail.com wrote:

Sounds great! Looking forward to it.

On Mon, Jun 4, 2018 at 9:46 AM Evan Jones notifications@github.com wrote:

I still need to merge and commit my pip_generate changes; let me try to get that merged today. Once that lands, I'd love feedback! In particular: This version should be faster to build; in particular it should be much faster to rebuild a target after you make a change. I'd love to know your experience.

I'd also like to add Python3 support to the CircleCI configuration so I don't break it inadvertently, so I'd love to know how you are using it.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/TriggerMail/rules_pyz/pull/10#issuecomment-394360298, or mute the thread https://github.com/notifications/unsubscribe-auth/ADtUNRfatDjvkBYUoIf3EeWwHguA0AAHks5t5TpBgaJpZM4UX09h .

evanj commented 6 years ago

Ooo this sounds great; I'll take a look at your fork and see how I can steal the relevant code; thanks!