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

rubyPackages/sass-embedded: init at 1.62.1 #233655

Closed

Conversation

Misterio77
Copy link
Member

@Misterio77 Misterio77 commented May 23, 2023

This gem is part of Jekyll's default dependency tree, meaning that newly locked bundix-managed jekyll projects will use it (and eventually rubyPackages.jekyll will probably, too, at some point).

It, however, will fail to build under nix, as its build step tries to fetch dart-sass-embedded from the internet. This PR adds rubyPackages.sass-embedded to nixpkgs and patches it to use nixpkgs' dart-sass-embedded, fixing the build.

Description of changes
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/)
  • 23.05 Release Notes (or backporting 22.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.

@Misterio77 Misterio77 requested a review from marsam as a code owner May 23, 2023 17:44
@Misterio77 Misterio77 force-pushed the ruby-package-sass-embedded branch from 883386d to d50d6a2 Compare May 23, 2023 19:15
@Misterio77 Misterio77 changed the title rubyPackages: add sass-embedded rubyPackages/sass-embedded: init at 1.62.1 May 23, 2023
@Misterio77 Misterio77 requested a review from AndersonTorres May 26, 2023 23:02
@Misterio77 Misterio77 force-pushed the ruby-package-sass-embedded branch from d50d6a2 to b1aed53 Compare May 30, 2023 22:38
This gem attempts to download dart-sass-embedded at build time, and that does not work with Nix. Patch it to find nixpkgs-provided dart-sass-embedded.
@Misterio77 Misterio77 force-pushed the ruby-package-sass-embedded branch from b1aed53 to 39c3a17 Compare May 30, 2023 22:39
@wegank wegank added 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 2.status: merge conflict This PR has merge conflicts with the target branch labels Mar 19, 2024
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 20, 2024
@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 4, 2024
Copy link
Contributor

@amiryal amiryal left a comment

Choose a reason for hiding this comment

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

Close this PR as stale.

@@ -697,6 +697,16 @@ in
buildFlags = [ "--disable-lto" ];
});

sass-embedded = _attrs: {
Copy link
Contributor

Choose a reason for hiding this comment

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

@AndersonTorres Re: #233655 (comment):
Hm… but throughout the rest of the same file (and throughout the rest of nixpkgs, AFAIK) this convention is not held. I would suggest to align with the prevailing custom and revert it back to attrs (no underscore).

Comment on lines +702 to +707
# Skip sass_embedded step, replace exe path with nixpkgs' dart-sass-embedded
postPatch = ''
substituteInPlace ext/sass/Rakefile \
--replace "'embedded.rb' => %w[sass_embedded] " "'embedded.rb'" \
--replace "sass_embedded/dart-sass-embedded" "${dart-sass-embedded}/bin/dart-sass-embedded"
'';
Copy link
Contributor

Choose a reason for hiding this comment

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

In later versions of the gem, these strings no longer appear. This PR really is stale and should be closed.

@stale stale bot removed 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md labels Oct 18, 2024
@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 2, 2025
@SuperSandro2000
Copy link
Member

Closing as stale

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.status: merge conflict This PR has merge conflicts with the target branch 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 6.topic: ruby 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 101-500 10.rebuild-linux: 101-500
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants