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

blender: implement multiple versions, move to fetchFromGitea #257780

Conversation

the-furry-hubofeverything
Copy link
Contributor

@the-furry-hubofeverything the-furry-hubofeverything commented Sep 28, 2023

Description of changes

Resolves #256845

I've changed the fetchurl to fetchFromGitea, to find the tagged sources easier (at the cost of download speed, I've found projects.blender.org isn't as fast as downloads.blender.org).

I ran into issues with applying the draco.patch to the addon repo, so I split it in two, patching each repo seperately.

I've roughly referenced pkgs/minecraft-servers/default.nix for my json to packages implementation, since I don't have much experience with nixpkgs. If that's not okay, please let me know.

I moved a lot of the original code, but I kept it the same for the most part. I'm new to writing for nixpkgs, so I'm only implementing the parts I know and a major refactor of the original code is not within scope.

  • Moved to fetchFromGitea
  • Implement version selection
  • Add isLTS attribute
  • Make update.py
  • Figure out blender-with-packages
  • Fix passthru test
  • Format code according to CONTRIBUTING.md
  • Added myself as maintainer
  • fix up packages dependent on blender-with-packages

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.11 Release Notes (or backporting 23.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@the-furry-hubofeverything the-furry-hubofeverything force-pushed the blender-improvements branch 2 times, most recently from a557861 to ce3313b Compare September 28, 2023 07:03
@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Sep 28, 2023

blender-with-packages = callPackage ../applications/misc/blender/wrapper.nix { };
blenderPackages = callPackage ../applications/misc/blender { };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blenderPackages is not the attribute naming scheme for versioning. Look at how it's done in e.g. boost.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to blender3_x, blender3_x-hip, and with blender, blender-lts, blender-hip etc

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So blender_3_x, blender-hip_3_x?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The _x is omitted.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The _x is omitted.

I don't understand what you mean.

The example in the link you gave me (https://nixos.org/manual/nixpkgs/stable/#sec-package-naming) with their example has json-c_0_9 and json-c_0_11.

If you mean blender_3x, personally I think that's confusing, given previously blender has versions like 2.93, and blender uses these to indicate API breaking changes (major and minor versions).

@the-furry-hubofeverything the-furry-hubofeverything force-pushed the blender-improvements branch 3 times, most recently from 62e050b to df7ac2c Compare October 2, 2023 08:52
@the-furry-hubofeverything
Copy link
Contributor Author

Almost ready. I just need to clean up the nix code.

@ofborg ofborg bot added 8.has: package (new) This PR adds a new package and removed 2.status: merge conflict This PR has merge conflicts with the target branch labels Oct 4, 2023
@the-furry-hubofeverything
Copy link
Contributor Author

the-furry-hubofeverything commented Oct 4, 2023

Almost ready. I just need to clean up the nix code.

Completely forgot wrapper.nix.

Okay, so, currently we have a couple of decisions to make.

Version naming conventions

So far I've gotten three options

all on top-level

Based on naming convention of boost, but using inherit (recurseIntoAttrs for specific versions. It has the advantage of not break overrides but it is expensive.
Also, all-packages.nix needs to update every minor version change.

  • blender
  • blender-lts
  • blender-hip
  • blender-hip-lts
  • blender_3_x
  • blender-hip_3_x

with blenderVersions(currently implmented)

Based on nixVersions, might remove blender-hip-lts and leave blender-hip only for backwards compatibility. Doesn't need changes to all-packages.nix for minor version updates. My preferred choice.

  • blender
  • blender-lts
  • blender-hip
  • blender-hip-lts
  • blenderVersions
    • blender_3_x
    • blender-hip_3_x
    • lts
    • stable

condensed blenderVersions

Users can define blenderVersions.blender_3_x.override { hipSupport = true; }; if they want it. Generates the least amount of package entries in blenderVersions.

  • blender
  • blender-lts
  • blender-hip
  • blender-hip-lts
  • blenderVersions
    • blender_3_x
    • lts
    • stable

blender-with-packages

From what I understand, blender-with-packages is a wrapper for Blender, but I don't understand how it works.

I assume it allows other packages to work with Blender, like using bpy. I think, at it's current state, it only works with the blender alias. Would it be feasible to generate a wrapper for each Blender version (blenderVersions.blender_3_x-with-packages)?

But then for existing packages in nixpkgs (i'm only aware of bpycv so far), how would they implement their package for multiple versions of Blender?

Opinions? @lucasew (for bpycv) @veprbl @pennae @cillianderoiste

@lucasew
Copy link
Contributor

lucasew commented Oct 4, 2023

blender-with-packages basically do what python.with-packages does. Blender 2.x still uses python 3.9 and 3.x python 3.10 and we had a problem with linking which python packages are supported for each blender version.

As far as I remember it takes blender as the first parameter and from a passthru attribute gets which python package set it supports (like python.pkgs).

@the-furry-hubofeverything
Copy link
Contributor Author

blender_3_3 doesn't pass render test,

@amarshall you wrote the unit test, I don't know much about command-line use in blender, do you think there's an argument change or something I've missed?

@lucasew
Copy link
Contributor

lucasew commented Oct 8, 2023

blender_3_3 doesn't pass render test,

@amarshall you wrote the unit test, I don't know much about command-line use in blender, do you think there's an argument change or something I've missed?

Which error?

@the-furry-hubofeverything
Copy link
Contributor Author

blender_3_3 doesn't pass render test,
@amarshall you wrote the unit test, I don't know much about command-line use in blender, do you think there's an argument change or something I've missed?

Which error?

I forgot to include the log, sorry

copying path '/nix/store/5g9nilk7v6lv4gipybskiv91q36k8ww9-libelf-0.8.13' from 'https://cache.nixos.org'...
copying path '/nix/store/nm7al9rc9a81q5y7p942xh96n2smbg1j-llvm-15.0.7-lib' from 'https://cache.nixos.org'...
copying path '/nix/store/qznjbcwwn39dnbd6rppr5gzsl2w916s3-libxshmfence-1.3.2' from 'https://cache.nixos.org'...
copying path '/nix/store/ja4ax8704yaap28a9v0xqbpx2ag8sckc-mesa-23.1.7' from 'https://cache.nixos.org'...
copying path '/nix/store/2j7b1ngdvqd0bidb6bn9icskwm6sq63v-perl-5.38.0' from 'https://cache.nixos.org'...
copying path '/nix/store/lf9pzvvg1y6kl4w89fh9cs73in0l5d39-stdenv-linux' from 'https://cache.nixos.org'...
copying path '/nix/store/17zxjnpgh8327zyfvbznn7p9r3nnykal-lm-sensors-3.6.0' from 'https://cache.nixos.org'...
copying path '/nix/store/swmp8c9a8indr65dkdmxjc1f26lgib40-mesa-23.1.7-drivers' from 'https://cache.nixos.org'...
Rendering with BLENDER_EEVEE...
Blender 3.3.11
could not get a list of mounted file-systems
GHOST/Wayland: error: XDG_RUNTIME_DIR is invalid or not set in the environment.
/nix/store/vmf7cqhrgk6fj2qdpjdx05szxkhpfx4a-blender-3.3.11/bin/blender() [0x33ef963]
/nix/store/9a9hfbv339z3jla9j59ql9q17wrs8sk1-wayland-1.22.0/lib/libwayland-client.so.0(+0xba1a) [0x7ffff7f21a1a]
/nix/store/9a9hfbv339z3jla9j59ql9q17wrs8sk1-wayland-1.22.0/lib/libwayland-client.so.0(wl_display_connect+0x2e7) [0x7ffff7f1dc07]
/nix/store/vmf7cqhrgk6fj2qdpjdx05szxkhpfx4a-blender-3.3.11/bin/blender() [0x1c5b35a]
/nix/store/vmf7cqhrgk6fj2qdpjdx05szxkhpfx4a-blender-3.3.11/bin/blender() [0x1c49aad]
/nix/store/vmf7cqhrgk6fj2qdpjdx05szxkhpfx4a-blender-3.3.11/bin/blender() [0x1c485d9]
/nix/store/vmf7cqhrgk6fj2qdpjdx05szxkhpfx4a-blender-3.3.11/bin/blender() [0xb298f1]
/nix/store/vmf7cqhrgk6fj2qdpjdx05szxkhpfx4a-blender-3.3.11/bin/blender() [0xb12e97]
/nix/store/vmf7cqhrgk6fj2qdpjdx05szxkhpfx4a-blender-3.3.11/bin/blender() [0xc30eb5]
/nix/store/vmf7cqhrgk6fj2qdpjdx05szxkhpfx4a-blender-3.3.11/bin/blender() [0x1963e09]
/nix/store/vmf7cqhrgk6fj2qdpjdx05szxkhpfx4a-blender-3.3.11/bin/blender() [0x19647c5]
/nix/store/vmf7cqhrgk6fj2qdpjdx05szxkhpfx4a-blender-3.3.11/bin/blender() [0x1968797]
/nix/store/vmf7cqhrgk6fj2qdpjdx05szxkhpfx4a-blender-3.3.11/bin/blender() [0x196ad50]
/nix/store/vmf7cqhrgk6fj2qdpjdx05szxkhpfx4a-blender-3.3.11/bin/blender() [0x196bb69]
/nix/store/vmf7cqhrgk6fj2qdpjdx05szxkhpfx4a-blender-3.3.11/bin/blender() [0x586292]
/nix/store/vmf7cqhrgk6fj2qdpjdx05szxkhpfx4a-blender-3.3.11/bin/blender() [0x3307dde]
/nix/store/vmf7cqhrgk6fj2qdpjdx05szxkhpfx4a-blender-3.3.11/bin/blender() [0x534434]
/nix/store/ld03l52xq2ssn4x0g5asypsxqls40497-glibc-2.37-8/lib/libc.so.6(+0x23ace) [0x7fffe9e3dace]
/nix/store/ld03l52xq2ssn4x0g5asypsxqls40497-glibc-2.37-8/lib/libc.so.6(__libc_start_main+0x89) [0x7fffe9e3db89]
/nix/store/vmf7cqhrgk6fj2qdpjdx05szxkhpfx4a-blender-3.3.11/bin/blender() [0x583ae5]
Unable to open a display
/build/.attr-0l2nkwhif96f51f4amnlf414lhl4rv9vh8iffyp431v6s28gsr90: line 19:     7 Aborted                 (core dumped) /nix/store/vmf7cqhrgk6fj2qdpjdx05szxkhpfx4a-blender-3.3.11/bin/blender --background -noaudio --factory-startup --python-exit-code 1 --python scene-config.py --engine "$engine" --render-output "$out/$engine" --render-frame 1

@the-furry-hubofeverything the-furry-hubofeverything removed 6.topic: module system About "NixOS" module system internals 6.topic: nim Nim programing language 6.topic: mate The MATE Desktop Environment 6.topic: vscode 6.topic: flakes The experimental Nix feature 6.topic: lib The Nixpkgs function library 6.topic: zig 6.topic: jupyter Interactive computing tooling: kernels, notebook, jupyterlab labels Dec 15, 2023
@the-furry-hubofeverything
Copy link
Contributor Author

Would it be easier to review this PR if it were to be split up to several smaller ones?

@SharzyL
Copy link
Contributor

SharzyL commented Dec 15, 2023

It seems that you need to rebase your commits

@the-furry-hubofeverything
Copy link
Contributor Author

the-furry-hubofeverything commented Dec 15, 2023

Dear lord- (didn't notice diff size)

Fixes patching issue. Blender-addons-contrib is not included, as decided upstream.
Git/Github seems to be confused for this rename, so a separate commit is needed.
moved derivation to generic.nix, passthrough `pythonPackages`

roughly referenced pkgs/minecraft-servers/default.nix

blender_3_3: init 3.3.14
blender_3_6: init 3.6.7
blender_4_0: init 4.0.2
moved derivation to generic.nix, passthrough `pythonPackages`

roughly referenced pkgs/minecraft-servers/default.nix

blender_3_3: init 3.3.14
blender_3_6: init 3.6.7
blender_4_0: init 4.0.2
@the-furry-hubofeverything
Copy link
Contributor Author

I'll split it up when I get the time to do so. Will set to draft (and maybe eventually close) since

  1. The two commits that need to be squashed ("blender: implement multi version packages")
  2. Blender 3.3 doesn't build on opencolorio 2.3 😧
  3. It's getting very big and messy

@the-furry-hubofeverything the-furry-hubofeverything marked this pull request as draft December 15, 2023 18:06
Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mostly formatting and design things


# TODO Consider following https://vfxplatform.com/ for dependency tracking
# Like "VFX_RefCY": "2024", which outlines python 3.11, qt 6.5+ etc
pythonPackages = python310Packages;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pythonPackages = python310Packages;
python3Packages = python310Packages;

lets make it clear that we are not using python2 here

, mesa
, runCommand
, callPackage
, version, hashes, isLTS, pythonPackages, xvfb-run
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
, version, hashes, isLTS, pythonPackages, xvfb-run
, version, hashes, isLTS, python3Packages, xvfb-run

}:

let
inherit (pythonPackages) python;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
inherit (pythonPackages) python;

we could avoid this if we would we using python3 instead of python3 pkgs.

Comment on lines +79 to +86
nativeBuildInputs =
[ cmake makeWrapper pythonPackages.wrapPython llvmPackages.llvm.dev
]
++ lib.optionals cudaSupport [
addOpenGLRunpath
cudaPackages.cuda_nvcc
]
++ lib.optionals waylandSupport [ pkg-config ];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
nativeBuildInputs =
[ cmake makeWrapper pythonPackages.wrapPython llvmPackages.llvm.dev
]
++ lib.optionals cudaSupport [
addOpenGLRunpath
cudaPackages.cuda_nvcc
]
++ lib.optionals waylandSupport [ pkg-config ];
nativeBuildInputs = [
cmake makeWrapper pythonPackages.wrapPython llvmPackages.llvm.dev
] ++ lib.optionals cudaSupport [
addOpenGLRunpath
cudaPackages.cuda_nvcc
] ++ lib.optionals waylandSupport [ pkg-config ];

lets make this a bit tidy

Comment on lines +87 to +88
buildInputs =
[ boost ffmpeg gettext glew ilmbase
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
buildInputs =
[ boost ffmpeg gettext glew ilmbase
buildInputs = [
boost ffmpeg gettext glew ilmbase

++ lib.optionals cudaSupport [ cudaPackages.cuda_cudart ]
++ lib.optional colladaSupport opencollada
++ lib.optional spaceNavSupport libspnav;
pythonPath = with pythonPackages; [ numpy requests zstandard ];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are using python3 as an input, we would need to use python3.pkgs here

pythonPath = with pythonPackages; [ numpy requests zstandard ];

postPatch = (if stdenv.isDarwin then ''
: > build_files/cmake/platform/platform_apple_xcode.cmake
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
: > build_files/cmake/platform/platform_apple_xcode.cmake
touch build_files/cmake/platform/platform_apple_xcode.cmake

touch is a bit more obvious

Comment on lines +146 to +148
cmakeFlags =
[
"-DWITH_ALEMBIC=ON"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
cmakeFlags =
[
"-DWITH_ALEMBIC=ON"
cmakeFlags = [
"-DWITH_ALEMBIC=ON"

'';

passthru = {
inherit python pythonPackages isLTS;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
inherit python pythonPackages isLTS;
inherit python isLTS;

we should only inherit python from where you can get pkgs via python.pkgs

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re: #257780 (review)

This was implemented over python3.pkgs as per #211340

echo "Rendering with $engine..."

# Blender doesn't support headless rendering on EEVEE before 3.4
if [ ${if lib.versionOlder version "3.4" then "0" else "1"} -eq 0 ] && [ $engine == "BLENDER_EEVEE" ]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if [ ${if lib.versionOlder version "3.4" then "0" else "1"} -eq 0 ] && [ $engine == "BLENDER_EEVEE" ]
if ${lib.boolToString (lib.versionOlder version "3.4")} && [ $engine == "BLENDER_EEVEE" ]

@amarshall amarshall mentioned this pull request Mar 5, 2024
13 tasks
@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Mar 20, 2024
@the-furry-hubofeverything
Copy link
Contributor Author

Closing, as this got really messy and it could use a bit of a clean up/rewrite

continuing with smaller, easier to review PRs

@the-furry-hubofeverything
Copy link
Contributor Author

Work continues on #305727

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.status: merge conflict This PR has merge conflicts with the target branch 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Having Blender stable and LTS versions