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

squeak: 4.10.2.2614 -> 5.3-19459 #133310

Merged
merged 2 commits into from Mar 22, 2022
Merged

Conversation

bb010g
Copy link
Member

@bb010g bb010g commented Aug 9, 2021

Credit to @BenBals for starting work on this patch in #86741. This package probably still needs more testing, but basic inisqueak execution works out of the box.

This patch doesn't use a newer release of the OpenSmalltalk VM because current Squeak doesn't use one. I don't know enough about Squeak to feel comfortable bumping the VM version.

Motivation for this change

inisqueak works in a fresh directory. I like that.

This patch tries to replicate the recommended sorta-bootstrapping arrangement for Squeak, as best as I can tell. It's not super well documented. https://github.com/squeak-smalltalk/squeak-app was a helpful reference.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • 21.11 Release Notes (or backporting 21.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.

pkgs/development/compilers/squeak/default.nix Outdated Show resolved Hide resolved
Comment on lines 6 to 9
, squeakImageVersion ? null, squeakImageHash ? null
, squeakSourcesHash ? null, squeakSourcesVersion ? null, squeakVersion ? null
, squeakVmVersion ? null, squeakVmVersionRelease ? null
, squeakVmCommitHash ? null, squeakVmCommitHashHash ? null
Copy link
Member

Choose a reason for hiding this comment

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

Do we really need this mess?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is the sanest way I could think of to not make version overrides require copying & pasting this source into a new expression.

Copy link
Member

Choose a reason for hiding this comment

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

This is the sanest way I could think of to not make version overrides require copying & pasting this source into a new expression.

you could put the "builder" in a separate nix file

Copy link
Member Author

@bb010g bb010g Aug 12, 2021

Choose a reason for hiding this comment

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

you could put the "builder" in a separate nix file

How does this allow overrides, though? If I have a ./generic.nix with an attrset parameter taking these values, returning a { lib, stdenv, ... }: stdenv.mkDerivation { ... } expression, that's most likely not going to be exported at top-level. It would probably be paired with a ./default.nix that is import ./generic.nix { squeakVersion = "5.3-19459"; ... }, and this doesn't address the concerns of overriding the version without having to paste the whole expression source. nixpkgs/lib's module system addresses this sort of customization. callPackage doesn't.

I considered doing something with makeScope, but that seemed like more of an annoyance than using a few more named arguments.

EDIT: I removed a few of the arguments where you'd probably already be copying & pasting the expression when changing anyways.

pkgs/development/compilers/squeak/default.nix Outdated Show resolved Hide resolved
Credit to @BenBals for starting work on this patch in
<NixOS#86741>.
@bb010g bb010g changed the title squeak: 4.10.2.2614 -> 5.0-202003021730 squeak: 4.10.2.2614 -> 5.3-19459 Aug 12, 2021
@ehmry ehmry mentioned this pull request Mar 22, 2022
10 tasks
@ofborg ofborg bot requested a review from ehmry March 22, 2022 15:15
@ehmry ehmry merged commit a3fcf30 into NixOS:master Mar 22, 2022
@bb010g bb010g deleted the f/squeak-opensmalltalk branch March 23, 2022 02:10
@ehmry
Copy link
Contributor

ehmry commented Mar 23, 2022

Everything is broken, I'm not sure if that's just the state of Smalltalk or a problem with the VM.

@bb010g
Copy link
Member Author

bb010g commented Mar 29, 2022

@ehmry What sorts of broken? My tests worked fine back around the time I opened the PR.

@ehmry
Copy link
Contributor

ehmry commented Mar 29, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants