Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: Rewriting in Rust #5

Closed
StandingPadAnimations opened this issue Jul 23, 2023 · 8 comments
Closed

Proposal: Rewriting in Rust #5

StandingPadAnimations opened this issue Jul 23, 2023 · 8 comments

Comments

@StandingPadAnimations
Copy link
Collaborator

As it stands, bpy-build's code is pretty... horrifying, and Python's standard library for file operations and CLI tools is not the best. Also, bpy-build is written to depend on Python 3.7 (since Blender 2.8 uses Python 3.7), which limits what we can do. In addition, I'm not the biggest fan on how Python libraries in general depend on very "dictionary-like" syntax. I also have a hard time understanding the code, and I wrote bpy-build.

Proposal: rewriting bpy-build in Rust. The reason for Rust is because Rust libraries are much better to deal with, and the build system is really easy (just cargo build). Rewriting in Rust would have the following benefits:

  • More readable code
  • Less headaches from Python libraries
  • Independence from Python version
  • The clap crate in Rust can generate tab completions, improving the CLI experience
  • Performance (though minor in this case)
  • Easily maintainability

The main disadvantages however would be:

  • Requiring binaries for each OS. Rust code is pretty much always cross platform (provided one doesn't use OS specific libraries), so the issue isn't really writing cross platform code, but rather making building bpy-build itself an additional step just to build MCprep.
    • This might be a non-issue if we take advantage of GitHub Actions, which I think is the way to go.
  • Rust is a completely different language from Python, making it harder for the other MCprep developers to understand what's going on. Personally I think with some good documentation, this issue can be mitigated.

For now I'll make a separate branch with the Rust rewrite, and let developers use it optionally, until we decide further on whether or not to fully replace bpy-build.

@TheDuckCow
Copy link
Member

Just copying over my comments from discord - apologies if this comes across wrong, I'm just trying to be direct:

As-is, I'm not really supportive of this move. I suppose you could argue there is little harm in building the system for us to later try and compare between the two, but as it stands I don't think the upsides are worth it (and would be wasted time, that could be put towards actual user-facing problems). Responding to the Pro's above:

The Pro arguments above are all pretty subjective in my mind:

More readable code

  • Readability is classically subjective, Id' say within any language you can establish the practices to make it more readable. Using a new language also requires (myself, at a minimum) the overhead to actually learn that language. Though that on its own isn't a good enough argument to not adopt something fit for the job, given the below points, it forces extra effort for others who aren't already familiar with rust.

Less headaches from Python libraries

  • I agree there could be much less overhead with the libraries that bpy-build has today, but I'd argue that was also somewhat self-inflicted as that's how bpy-build was made.
  • There's no reason we couldn't use lower level, more built in python to get the same job done, albeit with not quite as nice of an experience (I do appreciate the console build animations and such, they are nice; but if the argument is they add too much overhead, then that tradeoff should be considered).

Independence from Python version

  • It's a decent point, but we need to be able to execute the test fixture with python anyways.
  • We're not really escaping python with a python project, just managing the python version to install (and this is no different to rust itself, I would say, having a specific version or range of versions supported). Plus, we could potentially use workarounds to leverage the built in python environments from blender itself if it's really an issue (devs will already be supplying blender executable versions to run tests).

The clap crate in Rust can generate tab completions, improving the CLI experience

  • Nice, but feels quite minor - and I suppose it depends on the CLI? (unless are we talking about being having to enter an interactive rust environment? I would find that rather annoying and would end up writing wrapper scripts anyways to build).

Performance (though minor in this case)

  • Agreed, minor difference. The majority of time in both cases will be the zipping and copying files (including MCprep resources) into place, blowing out any split time processing benefits.

Easily maintainability

  • I would argue that having to maintain another language and coding standard will be way more overhead the the slight perceived dip of maintenance in the primary language. Better yet, why not just focus on how to reduce maintenance there instead. If we feel that bpy-build has too many dependencies (I wouldn't argue with that, given before compiling had no dependencies), then I'd say focus our efforts there to reduce it further.

As for the Cons, agree there - even if github actions works, it's still a whole lot of extra complexity and steps to maintain and manage for something that could just be a simpler solution. Remember, the original idea was actually to use make commands, but they were just barely not convenient enough to work with. I feel this would be swinging way too far in the other direction.

In the end, this is meant to build the addon in a cross compile way. Aside from some minor initial development fixes we are making around relative pathing, it basically does what we need and with minor to modest effort, can be extended to do more as needed in the future. But rewriting in rust seems like it would be a large amount of effort that would be better spent elsewhere, and an equal if not outsized ongoing effort to maintain that make this not worth the benefits it would provide.

I'm happy to discuss further, but wanted to be candid with how I feel about it at this stage.

@StandingPadAnimations
Copy link
Collaborator Author

StandingPadAnimations commented Jul 24, 2023

I'll clarify on the reasons why I'd prefer a Rust version, but I have some proposals to improve the development experience of the Python bpy-build should we decide against a Rust version.

Clarification on Rust

Performance overhead isn't the issue with libraries, the main headaches with libraries in Python are:

  • Lack of proper documentation.
  • Lack of type annotations for older ones (like PyYAML), which IDEs dislike.
  • Really clunky syntax.
  • Forced use of older dependencies for Python 3.7 support.

In addition, bpy-build uses the standard library for most operations, only using external dependencies for the following:

  • YAML parsing.
  • Arguments (I'm not a fan of argparse).
  • Console output.
  • Nice, but feels quite minor - and I suppose it depends on the CLI? (unless are we talking about being having to enter an interactive rust environment? I would find that rather annoying and would end up writing wrapper scripts anyways to build).

The current way bpy-build does CLI arguments is docopt (the cli parsing option I liked the most at the time), which has issues:

  • Unmaintained library, as with most of its forks.
  • Has its own syntax for CLI arguments.
  • Using arguments is again, clunky, with referencing into a dictionary (I'd rather have a class with member variables for arguments).

Bpy-build in my opinion requires a rewrite regardless (despite being extremely new software), even if we decide not to go with Rust, as it's already extemely verbose. Looking for the issue related to Zip file structure for instance took multiple weeks on my end due to certain confusing aspects related to shutil, and I haven't seen much better libraries for zipping files. In my opinion, the effort needed to debug a Python bpy-build (one bound to Python 3.7 as the minimum version) would be far greater then writing and maintaining a Rust version.

Python

All that being said, if we decide to not go ahead with a Rust version, I feel the following would need to be done with bpy-build to make it less of a headache to develop:

  • Bumped minimum version (like Python 3.9). Sticking with 3.7 has proven to be worse for bpy-build, and I'd assume most MCprep developers have a recent Python installed anyway (since modern IDEs expect it).
  • Swap YAML to TOML (this was planned anyway regardless). We do some horrific string regex currently that could be solved with how TOML does things, and TOML is more clear anyway.
  • Rewrite completely. Like I said, bpy-build is verbose and in my opinion would need to be rewritten anyway
  • A replacement library for argument parsing. As I've stated before, I'm not a fan of argparse, and the current library we use is unmaintained; we need to look into replacements
  • Actual strong type hinting. I can not put in words how much pain Python's dynamic nature adds to debugging (beyond it making me wish for C++'s type system and Clang's -Weverything flag).
  • Proper documentation for the libraries we use. Sadly (even with the standard library in certain cases), this is easier said the done.

Edit: Clize might be a good replacement for docopt

@StandingPadAnimations
Copy link
Collaborator Author

Here's the timeline for the rewrite (either Rust or Python, that'll still be open for discussion)

  • Fix remaining major bugs and ship 0.2.0 for MCprep 3.5
  • Begin Rust/Python rewrite (not bound to any MCprep version for now)
    • Replace YAML with TOML
    • Define bpy-build specification (partly documentation, partly to make bpy-build less of a black box)
    • Add fast option and watch command (auto-compile as files get changed)
    • (if Python) Require all builds to pass MyPy static analysis (MyPy will be a developer requirement, not for the final release). Bump minimum version to Python 3.8 or 3.9 (again, open for discussion)
    • Release bpy-build 0.3.0
  • (Problem for later, likely early to mid-2024) Move MCprep to the updated build system

@StandingPadAnimations
Copy link
Collaborator Author

StandingPadAnimations commented Jul 26, 2023

Alright based on how quiet this thread is, I'll start a Python rewrite, with Python 3.9 as the base

@StandingPadAnimations
Copy link
Collaborator Author

Alright I've pushed a base commit for all rewrites and added a Mypy config, with the following config:

  • No untyped functions
  • No expressions with the Any type (i.e. no dynamic typing)

Is it stricter then what most developers are used to? Yes
Would it prevent errors related to dynamic typing and make bpy-build more reliable? Also yes

I'll be making a contributer guide later this week

@TheDuckCow
Copy link
Member

Any changes that would yield differences for the MCprep repo? Or are these just incremental updates/rules.

Asking as, we've not even fully rolled out the current build version for MCprep, so if you're rewriting already (not that I think that's the best use of time..), should we just already adopt the new way before pushing others like zNight to get used to the current system?

@StandingPadAnimations
Copy link
Collaborator Author

The rewrite will impact configuration (YAML to TOML is planned, though now I'm playing around with YAML format improvements, particurally with during-build actions), but otherwise share the same CLI interface, so transitioning will be very minimal for the average developer.

(not that I think that's the best use of time..)

I'm not really a big fan of how verbose things have gotten, among other issues (especially with configuration), hence why this rewrite.

@TheDuckCow
Copy link
Member

Ok, I can certainly appreciate that and definitely support trying to reduce complexity. Thanks for taking it on and I'll be happy to review whenever something is ready (just be aware I may take a couple days at any given point to get to a code review, I sometimes need to take a break from code reviews after doing them all day!)

@StandingPadAnimations StandingPadAnimations closed this as not planned Won't fix, can't repro, duplicate, stale Dec 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants