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

openmoji-black,openmoji-color: fix hanging builds #174795

Merged
merged 1 commit into from
May 28, 2022

Conversation

fgaz
Copy link
Member

@fgaz fgaz commented May 26, 2022

Description of changes

Fixes #167869

ZHF: #172160
@NixOS/nixos-release-managers

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

@fgaz fgaz added the 0.kind: build failure A package fails to build label May 26, 2022
];
buildInputs = old.buildInputs ++ [ libuninameslist ];
});
scfbuild-with-pinned-fontforge = scfbuild.override (old: {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: It seems weird to call one the date and one just "pinned". Maybe just call both -pinned?

Copy link
Member Author

Choose a reason for hiding this comment

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

I prefer using the version then. Fixed.

@ofborg ofborg bot added 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels May 26, 2022
@kevincox
Copy link
Contributor

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

2 packages built:
  • openmoji-black
  • openmoji-color

Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

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

Please keep the original names so that we don't mix two versions together.

@@ -26,7 +57,7 @@ in stdenv.mkDerivation rec {
};

nativeBuildInputs = [
scfbuild
scfbuild-with-fontforge-20201107
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
scfbuild-with-fontforge-20201107
scfbuild

Comment on lines +44 to +46
scfbuild-with-fontforge-20201107 = scfbuild.override (old: {
fontforge = fontforge-20201107;
});
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
scfbuild-with-fontforge-20201107 = scfbuild.override (old: {
fontforge = fontforge-20201107;
});
scfbuild = scfbuild.override (old: {
inherit fontforge;
});

# https://github.com/NixOS/nixpkgs/issues/167869
# Patches etc taken from
# https://github.com/NixOS/nixpkgs/commit/69da642a5a9bb433138ba1b13c8d56fb5bb6ec05
fontforge-20201107 = fontforge.overrideAttrs (old: rec {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
fontforge-20201107 = fontforge.overrideAttrs (old: rec {
fontforge = fontforge.overrideAttrs (old: rec {

Copy link
Member Author

Choose a reason for hiding this comment

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

isn't that infinite recursion since in nix let is recursive?

Copy link
Member

Choose a reason for hiding this comment

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

🤔 could be. I am not sure right now.

Then normally something like the following is used

Suggested change
fontforge-20201107 = fontforge.overrideAttrs (old: rec {
fontforge' = fontforge.overrideAttrs (old: rec {

Copy link
Contributor

Choose a reason for hiding this comment

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

If the shadowing doesn't work I would rather stick with meaningful names rather than '.

@risicle
Copy link
Contributor

risicle commented May 28, 2022

nixpkgs-review happy, macos 10.15

@github-actions
Copy link
Contributor

Successfully created backport PR #175190 for release-22.05.

@fgaz fgaz deleted the openmoji/pin-fontforge branch May 29, 2022 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.kind: build failure A package fails to build 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

openmoji has started taking forever to build.
4 participants