factor-lang: Restructure package for easier extension

[spacefrogg]

Description of changes

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
  • sandbox = relaxed
  • sandbox = true
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• 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/)
• 24.05 Release Notes (or backporting 23.05 and 23.11 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.

---

Add a 👍 reaction to pull requests you find important.

[nixos-discourse]

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

https://discourse.nixos.org/t/prs-ready-for-review/3032/3438

[spacefrogg]

I have also added a forgotten feature buildFactorApplication and added proper documentation in the Nixpkgs manual.

[spacefrogg]

Regarding the change in CODEOWNERS. I would like to take the responsibility but, obviously, need some kind of higher-up approval. I can also revert the change again.

[SuperSandro2000]

Regarding the change in CODEOWNERS. I would like to take the responsibility but, obviously, need some kind of higher-up approval. I can also revert the change again.

The problem with codeowners is, that it doesn't properly work if you do not have merge permissions :(

[SuperSandro2000]

lots of nits, but otherwise looking good

[nixos-discourse]

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

https://discourse.nixos.org/t/prs-ready-for-review/3032/3696

[spacefrogg]

Most build helpers would respect user-specified phases. For example, this is how buildGoModule provides its buildPhase:

https://github.com/NixOS/nixpkgs/blob/master/pkgs/build-support/go/module.nix#L202

Are there solid reasons to prohibit users from specifying the buildPhase and the installPhase?

This is resolved.

[nixos-discourse]

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

https://discourse.nixos.org/t/prs-ready-for-review/3032/4919

[spacefrogg]

@ShamrockLee Polite ping, as you've requested changes (which I addressed), which might deter other reviewers with commit access to move this PR forward.

[ShamrockLee]

@spacefrogg, thank you for being so patient, and I'm sorry for not replying for so long. I'll review this PR tomorrow afternoon (after the final exam).

[spacefrogg]

@spacefrogg, thank you for being so patient, and I'm sorry for not replying for so long. I'll review this PR tomorrow afternoon (after the final exam).

Don't you worry! Most of us do this in our spare time. I wish you all the best for your exam! Thank you for looking into this.

[ShamrockLee]

It looks like mkFactorVocabulary and the package it returns don't depend on the factor compiler.

Then why do we need another top-level package set, factorPackages, for the vocabulary packages? Would it be easier to place the Factor vocabularies inside pkgs/by-name?

[spacefrogg]

Right now, the design shall help to make them easier to discover. Compare it to Texlive packages, where the simplest ones also do not depend on the TeX compiler but just provide a set of source files to be used by the end-user of the Texlive distribution. This is just a consequence of languages (like Factor or TeX) that have no notion of pre-compiled libraries but compile everything from source at the very end.

Additionally, Factor (in contrast to TeX) might add such a notion (like Python or Haskell) and then we don't need to change the package organisation again.

EDIT:
Factor vocabularies (like Texlive packages) are not meant to be used by end-users but are there to compile a Factor environment with the right set of vocabularies available (just like in the Texlive case). So, it might be additionally confusing to put them under pkgs/by-name, where they are of no immediate use.

[ShamrockLee]

Discussions/suggestions about some other design choices.

[ShamrockLee]

None-blocking nits

All blocking issues are addressed.

[spacefrogg]

@ShamrockLee Could I convince you to leave a positive review? :)

[ShamrockLee]

Just a few minutes. I'm building painter locally.

@ofborg build factorPackages.bresenham

[ShamrockLee]

Remaining bits of suggestions. Other than that, LGTM.

[ShamrockLee]

I read through the code and built the painter Factor application mentioned in the documentation; all looks good.

It would still be great if we could eventually stop depending on the directory layout and the name of the unpacked source directory, but that probably requires the upstream support of out-of-tree vocabulary to become mature (as discussed in #287852 (comment)). For now, let's make Factor packaging possible by merging this.