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

bun: init at 0.1.1 #180299

Merged
merged 1 commit into from
Jul 9, 2022
Merged

bun: init at 0.1.1 #180299

merged 1 commit into from
Jul 9, 2022

Conversation

DAlperin
Copy link
Member

@DAlperin DAlperin commented Jul 6, 2022

Description of changes

Add a package for https://bun.sh

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.

@DAlperin DAlperin force-pushed the add-bun branch 3 times, most recently from 224fec1 to 57f691e Compare July 6, 2022 21:44
@DAlperin DAlperin requested a review from sudosubin July 7, 2022 00:05
@DAlperin
Copy link
Member Author

DAlperin commented Jul 7, 2022

Alright @sudosubin that should do it I think

@Kranzes
Copy link
Member

Kranzes commented Jul 7, 2022

Why are we not building this from source?

@DAlperin
Copy link
Member Author

DAlperin commented Jul 7, 2022

For one, the nature of bun makes that very difficult to do. But besides that zig on Darwin is broken which would make that impossible to build for now. Additionally @sudosubin pointed out:

And bun uses patched webkit on linux during the build by zig, and produces cross-platform outputs. I'm not sure the webkit can be built on darwin with nix.

@DAlperin
Copy link
Member Author

DAlperin commented Jul 7, 2022

It's probably possible and a good idea to get it building under nix but I think that would be a not insignificant undertaking over a not insignificant amount of time.

@06kellyjac
Copy link
Member

Yeah I've been playing with a source build and it's a bit of a faff

Copy link
Member

@06kellyjac 06kellyjac left a comment

Choose a reason for hiding this comment

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

the vast majority of this looks shared across darwin and linux

pkgs/development/web/bun/darwin.nix Outdated Show resolved Hide resolved
pkgs/development/web/bun/darwin.nix Outdated Show resolved Hide resolved
pkgs/development/web/bun/darwin.nix Outdated Show resolved Hide resolved
pkgs/development/web/bun/default.nix Outdated Show resolved Hide resolved
pkgs/development/web/bun/default.nix Outdated Show resolved Hide resolved
pkgs/development/web/bun/default.nix Outdated Show resolved Hide resolved
@DAlperin
Copy link
Member Author

DAlperin commented Jul 7, 2022

Alrighty folks, hows this?

pkgs/top-level/all-packages.nix Show resolved Hide resolved
pkgs/development/web/bun/default.nix Outdated Show resolved Hide resolved
@DAlperin DAlperin force-pushed the add-bun branch 4 times, most recently from 2da35a6 to 2c47474 Compare July 8, 2022 03:44
Copy link
Member

@06kellyjac 06kellyjac left a comment

Choose a reason for hiding this comment

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

Looking very good. I'd advise a handful of minor tweaks

pkgs/development/web/bun/default.nix Outdated Show resolved Hide resolved
pkgs/development/web/bun/default.nix Outdated Show resolved Hide resolved
pkgs/development/web/bun/default.nix Outdated Show resolved Hide resolved
pkgs/development/web/bun/default.nix Outdated Show resolved Hide resolved
Copy link

@leap0x7b leap0x7b left a comment

Choose a reason for hiding this comment

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

Can't you just build it from source instead of just downloading the binaries from GitHub releases?

@DAlperin
Copy link
Member Author

DAlperin commented Jul 8, 2022

@leapofazzam123, this was discussed previously in the resolved convos in the thread. TL;DR: it is extremely complex and difficult to build, nix doesn't have the ability to build it native on darwin right now (zig is marked broken on darwin), and the build relies on cross compilation in a way that does not lend itself to being built from source right now. Thusly @06kellyjac @sudosubin myself and other decided it was best to use the binaries for now.

@ofborg ofborg bot requested a review from 06kellyjac July 8, 2022 17:19
@06kellyjac
Copy link
Member

Yeah. We can always swap to a source build once we get it working but it is a mess getting all the pieces together

@VergeDX
Copy link
Member

VergeDX commented Jul 9, 2022

Should we use the bun-bin pname until someone build it from source?

@DAlperin
Copy link
Member Author

DAlperin commented Jul 9, 2022

Personally I think that makes things more confusing then necessary for people looking for the bun package. Bun is a new project that will take some real for it's build system etc to get more polished. Once it becomes more possible down the line, I don't think it's that big of a deal to rewrite the derivation, but again I think it might be a bit before that's even possible and or reasonable.

@06kellyjac
Copy link
Member

I dont think there's much of a standard here. I think the source provenance marking it as a binary is fine for now 🤷

pkgs/development/web/bun/default.nix Outdated Show resolved Hide resolved
pkgs/development/web/bun/default.nix Outdated Show resolved Hide resolved
@06kellyjac
Copy link
Member

@lucasew 's suggestions plus a license tweak

diff --git a/pkgs/development/web/bun/default.nix b/pkgs/development/web/bun/default.nix
index 1a9c16d587b..04124379b14 100644
--- a/pkgs/development/web/bun/default.nix
+++ b/pkgs/development/web/bun/default.nix
@@ -30,16 +30,16 @@ stdenvNoCC.mkDerivation rec {
     sha256 = dist.sha256;
   };
 
-  sourceRoot = ".";
-  unpackCmd = "unzip bun-${dist.shortName}-${dist.arch}.zip";
+  strictDeps = true;
+  nativeBuildInputs = [ unzip ] ++ lib.optionals stdenvNoCC.isLinux [ autoPatchelfHook ];
+  buildInputs = [ openssl ];
+
   dontConfigure = true;
   dontBuild = true;
-  nativeBuildInputs = if stdenvNoCC.isLinux then [ autoPatchelfHook ] else [];
-  buildInputs = [ unzip openssl ];
 
   installPhase = ''
     runHook preInstall
-    install -Dm 755 ./bun-${dist.shortName}-${dist.arch}/bun $out/bin
+    install -Dm 755 ./bun $out/bin/bun
     runHook postInstall
   '';
 
@@ -52,8 +52,8 @@ stdenvNoCC.mkDerivation rec {
       All in one fast & easy-to-use tool. Instead of 1,000 node_modules for development, you only need bun.
     '';
     license = with licenses; [
-      mit #bun core
-      lgpl2 #javascriptcore and webkit
+      mit # bun core
+      lgpl21Only # javascriptcore and webkit
     ];
     maintainers = with maintainers; [ DAlperin jk ];
     platforms = builtins.attrNames dists;

@DAlperin
Copy link
Member Author

DAlperin commented Jul 9, 2022

@06kellyjac applied the patch, thank you! @lucasew take a look. Think we can get this merged?
Appreciate the feedback from all on the best way to package this for the time being.


src = fetchurl {
url = "https://github.com/Jarred-Sumner/bun-releases-for-updater/releases/download/bun-v${version}/bun-${dist.shortName}-${dist.arch}.zip";
sha256 = dist.sha256;
Copy link
Contributor

Choose a reason for hiding this comment

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

inherit (dist) sha256;

dist = dists.${stdenvNoCC.hostPlatform.system} or (throw "Unsupported system: ${stdenvNoCC.hostPlatform.system}");
in
stdenvNoCC.mkDerivation rec {
version = "0.1.1";
Copy link
Contributor

Choose a reason for hiding this comment

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

The latest version right now is 0.1.2

Copy link
Contributor

@Mindavi Mindavi left a comment

Choose a reason for hiding this comment

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

Whenever ofborg is happy good to go IMO. No need to update to 0.1.2 in this PR, you can do that in a follow up

@DAlperin
Copy link
Member Author

DAlperin commented Jul 9, 2022

@Mindavi ofborg seems happy. Can we go ahead and merge. I can update after in a quick follow up.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 9, 2022

Successfully created backport PR #180900 for release-22.05.

@Janik-Haag Janik-Haag added the 12. first-time contribution This PR is the author's first one; please be gentle! label Jun 12, 2023
@cdmistman cdmistman mentioned this pull request Jul 22, 2023
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 10.rebuild-linux: 1 11.by: package-maintainer This PR was created by the maintainer of the package it changes 12. first-time contribution This PR is the author's first one; please be gentle!
Projects
None yet
Development

Successfully merging this pull request may close these issues.