Skip to content

optiland: init at 0.6.0#509199

Merged
doronbehar merged 2 commits into
NixOS:masterfrom
doronbehar:pkg/optiland
Apr 24, 2026
Merged

optiland: init at 0.6.0#509199
doronbehar merged 2 commits into
NixOS:masterfrom
doronbehar:pkg/optiland

Conversation

@doronbehar
Copy link
Copy Markdown
Contributor

Things done

  • Built on platform:
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Tested, as applicable:
  • Ran nixpkgs-review on this PR. See nixpkgs-review usage.
  • Tested basic functionality of all binary files, usually in ./result/bin/.
  • Nixpkgs Release Notes
    • Package update: when the change is major or breaking.
  • NixOS Release Notes
    • Module addition: when adding a new NixOS module.
    • Module update: when the change is significant.
  • Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

@nixpkgs-ci nixpkgs-ci Bot requested a review from natsukium April 12, 2026 07:53
@nixpkgs-ci nixpkgs-ci Bot added 8.has: package (new) This PR adds a new package 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 6.topic: python Python is a high-level, general-purpose programming language. labels Apr 12, 2026
@doronbehar doronbehar requested a review from SFrijters April 12, 2026 11:44
@SFrijters
Copy link
Copy Markdown
Member

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 509199
Commit: 1ed6ce7c0312898ec60ea2a5eb152087a6c6344f


x86_64-linux

✅ 6 packages built:
  • optiland
  • optiland.dist
  • python313Packages.optiland
  • python313Packages.optiland.dist
  • python314Packages.optiland
  • python314Packages.optiland.dist

@doronbehar
Copy link
Copy Markdown
Contributor Author

Thanks @SFrijters. I was mostly wondering, what did you think of the withOptionalComponents design choice? Is it to generalizing? Other alternatives I've considered:

  • Use a withGui and withTorch arguments.
  • Make the top-level optiland attribute be a python3.withPackages environment, that will use python3.pkgs.optiland as is, and add the python3.pkgs.optiland.passthru.optional-dependencies attributes as needed.

What do you think? Currently, the optiland and python3.pkgs.optiland are two different derivations due to the additional dependencies added to the former.

@SFrijters
Copy link
Copy Markdown
Member

Without knowing anything about the project itself: withOptionalComponents feels a bit too generalized for me if there are only two possible components. In general I don't like strings much since the discoverability of valid values is not great - if we could have a list of some sum type that would feel different.

In this case I would probably drop the full customization completely and use your second alternative and explicitly override dependencies to add python3.pkgs.optiland.passthru.optional-dependencies.{gui, torch} inside the top-level application.

@doronbehar
Copy link
Copy Markdown
Contributor Author

In this case I would probably drop the full customization completely and use your second alternative and explicitly override dependencies to add python3.pkgs.optiland.passthru.optional-dependencies.{gui, torch} inside the top-level application.

The 2nd alternative is building an environment that will use the existing python3.pkgs.optiland package and wrap the executables of it with Qt and other python dependencies - not overriding python3.pkgs.optiland with the additional dependencies.

I wrote something using buildEnv, your review will be appreciated. If this is good in your opinion I will squash the last commit to the optiland: init commit.

@doronbehar
Copy link
Copy Markdown
Contributor Author

doronbehar commented Apr 14, 2026

@doronbehar doronbehar marked this pull request as draft April 14, 2026 12:07
@doronbehar doronbehar changed the base branch from master to staging-next April 14, 2026 12:07
@doronbehar doronbehar marked this pull request as ready for review April 14, 2026 12:08
@nixpkgs-ci nixpkgs-ci Bot closed this Apr 14, 2026
@nixpkgs-ci nixpkgs-ci Bot reopened this Apr 14, 2026
@doronbehar
Copy link
Copy Markdown
Contributor Author

nixpkgs-vet issues are solved :).

@SFrijters
Copy link
Copy Markdown
Member

I want to take a look at the final layout of the derivation (the new definition with buildEnv adds some bits I'm not familiar with) and how the wrapper looks, but my build server was busy today running other reviews - I'll try to take a look tomorrow.

Comment thread pkgs/by-name/op/optiland/package.nix
@SFrijters
Copy link
Copy Markdown
Member

The final result contains a lot of stuff (lib, include, links to shiboken6 and PySide6) that I would expect to be covered by the wrapper or the underlying python derivation, but I guess that's expected / a misconception on my end?

I've been able to run it through, so it seems to work :)

@nixpkgs-ci nixpkgs-ci Bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Apr 16, 2026
@doronbehar
Copy link
Copy Markdown
Contributor Author

The final result contains a lot of stuff (lib, include, links to shiboken6 and PySide6) that I would expect to be covered by the wrapper or the underlying python derivation, but I guess that's expected / a misconception on my end?

It's a good question, we could remove these links with a more minimal pathsToLink, or we could make this package contain only $out/bin/optiland, and wrap it as wrapped already. I think it will be a bit better to not pollute user's systems with general Python libraries. What do you think?

@doronbehar
Copy link
Copy Markdown
Contributor Author

OK I convinced myself it is better to do it this way. Now commit log is more concise, and top-level's optiland/package.nix expression is also using stdenvNoCC.mkDerivation.

@doronbehar
Copy link
Copy Markdown
Contributor Author

I think I will wait for #507470 to reach master, and then will test optiland at least builds on Darwin.

@doronbehar doronbehar requested a review from SFrijters April 17, 2026 12:58
@doronbehar
Copy link
Copy Markdown
Contributor Author

Waiting for the following comment of mine to be addressed:

@SFrijters
Copy link
Copy Markdown
Member

The results looks a lot cleaner now, I like it.

@vcunat vcunat changed the base branch from staging-next to master April 22, 2026 13:25
@nixpkgs-ci nixpkgs-ci Bot closed this Apr 22, 2026
@nixpkgs-ci nixpkgs-ci Bot reopened this Apr 22, 2026
@doronbehar doronbehar marked this pull request as draft April 23, 2026 07:29
@doronbehar
Copy link
Copy Markdown
Contributor Author

Converted to a draft because I wish to wait for upstream's fix for the Qt wayland issue.

@doronbehar
Copy link
Copy Markdown
Contributor Author

On a 2nd thought submitting an upstream PR and fetchpatch it is better.

@doronbehar doronbehar marked this pull request as ready for review April 23, 2026 08:00
@doronbehar
Copy link
Copy Markdown
Contributor Author

Will still give upstream a day or two to reply, and if not, merge it afterwards.

@doronbehar doronbehar enabled auto-merge April 24, 2026 09:59
@doronbehar doronbehar added this pull request to the merge queue Apr 24, 2026
Merged via the queue into NixOS:master with commit 684cd51 Apr 24, 2026
26 checks passed
@doronbehar doronbehar deleted the pkg/optiland branch April 24, 2026 10:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: python Python is a high-level, general-purpose programming language. 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants