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.

Sign up for GitHub

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

Jump to bottom

Cjs upgrade #99008

Merged
merged 3 commits into from Sep 30, 2020
Merged

Cjs upgrade #99008

merged 3 commits into from Sep 30, 2020

Conversation

mkg20001
Copy link
Member

@mkg20001 mkg20001 commented Sep 28, 2020 •
edited

Motivation for this change

Upgrade cjs with a patch to use newer mozjs

// cc @worldofpeace @doronbehar I took the spidermonkey 78 commit from the GNOME 3.38 pr

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@mkg20001 mkg20001 mentioned this pull request Sep 28, 2020
Closed
@ofborg ofborg bot requested a review from abbradar September 28, 2020 14:13
doronbehar
doronbehar reviewed Sep 29, 2020
View reviewed changes
pkgs/desktops/cinnamon/cjs/default.nix
owner = "linuxmint";
owner = "leigh123linux";
Copy link
Contributor

Choose a reason for hiding this comment

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

What's wrong with upstream?

Copy link
Contributor

Choose a reason for hiding this comment

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

You should grab linuxmint/cjs#84 as a patch.

Copy link
Member Author

Choose a reason for hiding this comment

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

The patch is quite gigantic. There have been some problems with renames not being applied in previous patches I applied, so I decided to use that approach as it seems saner.

@doronbehar
Copy link
Contributor

@worldofpeace are we interested in having a ./generic.nix for all the spidermonkey derivations?

@worldofpeace
Copy link
Contributor

@worldofpeace are we interested in having a ./generic.nix for all the spidermonkey derivations?

It would be best to not duplicate them. But I think at some point they diverged greatly, and there's already code committed here. So I would say a bug for "later". And, yeah, I've thought about this too.

@worldofpeace worldofpeace merged commit 3451caf into NixOS:master Sep 30, 2020
@quyse
Copy link
Contributor

quyse commented Oct 26, 2020

gjs-1.66.0 of leigh123linux/cjs repo is a branch, not a tag, and there were a few commits to that branch since then. At the moment fresh fetching of cjs source is broken because of wrong sha256: it should be now 00a7zq5vvz65n18apdzqaai4pjzv26zybkxdspy17xcdyl9rk497. rev should probably be pointed to a specific commit, not a branch?

@mkg20001
Copy link
Member Author

Oh yes, my bad

@mkg20001 mkg20001 deleted the cjs-upgrade branch October 26, 2020 10:44
@worldofpeace
Copy link
Contributor

Oh goodness didn't notice that. It would be cool if CI checks wouldn't allow such unstable references in fetchers.

@worldofpeace worldofpeace mentioned this pull request Oct 26, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@doronbehar doronbehar doronbehar left review comments

@worldofpeace worldofpeace worldofpeace approved these changes

@abbradar abbradar Awaiting requested review from abbradar

Assignees
No one assigned
Labels
6.topic: cinnamon 8.has: package (new) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@mkg20001 @doronbehar @worldofpeace @quyse