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

perlPackages.TclpTk: init at 1.09 #175775

Merged
merged 4 commits into from
Jun 6, 2022
Merged

perlPackages.TclpTk: init at 1.09 #175775

merged 4 commits into from
Jun 6, 2022

Conversation

willcohen
Copy link
Contributor

@willcohen willcohen commented Jun 1, 2022

Description of changes

Adds Tcl::pTk perl package, with necessary dependencies included as well. Builds on darwin and linux, though GUI functionality of Tcl::pTk only tested on darwin.

Note to @agbrooks -- as you noted in #122472, my previous snippet for perlPackages.Tcl didn't work on linux. I had to update the makeMakerFlags to include tclsh and disable stubs. I still struggled with the tk issue noted there, and opted instead to add Tcl::pTk instead of Tcl::Tk for now.

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/)
  • 22.11 Release Notes (or backporting 22.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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@willcohen willcohen changed the title perlPackages.TclpTk: init at 1.0.9 perlPackages.TclpTk: init at 1.09 Jun 1, 2022
@ofborg ofborg bot requested a review from agbrooks June 1, 2022 16:55
@willcohen willcohen changed the title perlPackages.TclpTk: init at 1.09 [WIP] perlPackages.TclpTk: init at 1.09 Jun 1, 2022
@willcohen willcohen marked this pull request as draft June 1, 2022 20:55
@willcohen willcohen force-pushed the tclptk branch 2 times, most recently from 7703ad3 to 6771e30 Compare June 1, 2022 21:39
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/linking-a-compiled-library-for-perlpackages/19465/1

@willcohen
Copy link
Contributor Author

Result of nixpkgs-review pr 175775 run on x86_64-linux 1

10 packages built:
  • perl532Packages.Tcl
  • perl532Packages.TclpTk
  • perl532Packages.TextLevenshteinXS
  • perl532Packages.TkToolBar
  • perl532Packages.WebServiceValidatorHTMLW3C
  • perl534Packages.Tcl
  • perl534Packages.TclpTk
  • perl534Packages.TextLevenshteinXS
  • perl534Packages.TkToolBar
  • perl534Packages.WebServiceValidatorHTMLW3C

@willcohen willcohen changed the title [WIP] perlPackages.TclpTk: init at 1.09 perlPackages.TclpTk: init at 1.09 Jun 2, 2022
@willcohen willcohen marked this pull request as ready for review June 2, 2022 18:51
@willcohen
Copy link
Contributor Author

@agbrooks sorry for the noise here. Turns out I just needed a specific incantation of makeMakerFlags for Tcl to work on nixos.

@willcohen
Copy link
Contributor Author

Result of nixpkgs-review pr 175775 run on x86_64-darwin 1

11 packages built:
  • bwidget
  • perl532Packages.Tcl
  • perl532Packages.TclpTk
  • perl532Packages.TextLevenshteinXS
  • perl532Packages.TkToolBar
  • perl532Packages.WebServiceValidatorHTMLW3C
  • perl534Packages.Tcl
  • perl534Packages.TclpTk
  • perl534Packages.TextLevenshteinXS
  • perl534Packages.TkToolBar
  • perl534Packages.WebServiceValidatorHTMLW3C

@zakame
Copy link
Member

zakame commented Jun 6, 2022

Result of nixpkgs-review pr 175775 run on aarch64-darwin 1

11 packages built:
  • bwidget
  • perl532Packages.Tcl
  • perl532Packages.TclpTk
  • perl532Packages.TextLevenshteinXS
  • perl532Packages.TkToolBar
  • perl532Packages.WebServiceValidatorHTMLW3C
  • perl534Packages.Tcl
  • perl534Packages.TclpTk
  • perl534Packages.TextLevenshteinXS
  • perl534Packages.TkToolBar
  • perl534Packages.WebServiceValidatorHTMLW3C

@stigtsp
Copy link
Member

stigtsp commented Jun 6, 2022

@GrahamcOfBorg build perlPackages.TclpTk

Copy link
Member

@stigtsp stigtsp left a comment

Choose a reason for hiding this comment

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

Hi there!

TextLevenshteinXS and WebServiceValidatorHTMLW3C do not seem to be dependencies of TclpTk afaik. Would it be better to set up separate PRs for those?

@willcohen
Copy link
Contributor Author

Of course! I was just trying to avoid PR churn, but I've pulled out those two as #176550 and #176551 to keep the PRs cleaner. (For what it's worth, this and those two are related only in that I'm trying to create a build environment for https://github.com/DistributedProofreaders/guiprep/).

@stigtsp
Copy link
Member

stigtsp commented Jun 6, 2022

@GrahamcOfBorg build perlPackages.TclpTk perlPackages.TkToolbar perlPackages.Tcl bwidget

@willcohen
Copy link
Contributor Author

@stigtsp the only consideration that's not fully resolved is the use of bwidget and tk in Tcl. Neither are actually hard dependencies of Tcl, but pTk projects may need both depending on the set of widgets they utilize (see #122472). Since this list could very well grow if/when more Tcl-dependent packages are added, an option might be to have just Tcl be vanilla with no additional tcl packages, and have something like TclFull be the growing list, since I'm not confident that these two additional packages are the only ones that might want to be added.

The reason I didn't go down this route already is since no one has to date had a use for such a vanilla Tcl package for perl, so it seemed like it might be a premature optimization.

stigtsp
stigtsp previously approved these changes Jun 6, 2022
Copy link
Member

@stigtsp stigtsp left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@stigtsp stigtsp dismissed their stale review June 6, 2022 20:48

Missed out on some debug in buildPhase

@stigtsp
Copy link
Member

stigtsp commented Jun 6, 2022

@GrahamcOfBorg build perlPackages.TclpTk perlPackages.TkToolbar perlPackages.Tcl

@stigtsp stigtsp merged commit f46e56d into NixOS:master Jun 6, 2022
@willcohen
Copy link
Contributor Author

Thank you very much!

@willcohen willcohen deleted the tclptk branch June 6, 2022 22:40
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

4 participants