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
cinnamon.cjs: init at 4.4.0 #75680
cinnamon.cjs: init at 4.4.0 #75680
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jtojnar and I discussed how it's pretty terrible this needs an old spidermonkey version.
Let's not readd it globally. Please move that expression into the cjs
directory and do
let
# https://github.com/linuxmint/cjs/issues/80
spidermonkey_52 = callPackage ./spidermonkey_52.nix {};
in
Here's what's in cjs's .pc file btw
|
17967ac
to
4d7d1d4
Compare
81b1995
to
f934b7b
Compare
#75680 (comment) and #75680 (comment) still stand. I don't see references to some of those deps like upower https://github.com/linuxmint/cjs/search?q=upower&unscoped_q=upower. Or maybe the GitHub search isn't working. |
7ea4540
to
042271c
Compare
I've updated the dependencies to include those that are provided by the .pc file I've left in the others, since otherwise I'd get some errors when trying to start the sessions. Also, I'm using this on my lap so I try to not break it 😅 |
gnome3.caribou | ||
keybinder3 | ||
upower | ||
xapps |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where are these needed in CJS? I'm thinking maybe it's their typelib's in GI_TYPELIB_PATH
are what's needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I suppose that's it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I keep them? Is there a way to just add them into the typelib path?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What error does it produce when omitted?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a .js file does things like require("xapps")
it complains that it's typelib can't be found
Though I don't know if the typelibs should be added here or wrapped/added via env in another package
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the error occurs on importing xapps, the packages should be added to propagateBuildInputs
of xapps
and wrapGAppsHook
should be added to the application importing xapps.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not just xapps, all the others as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well every runtime dependency that is not resolved at build time (e.g. hardcoded by a patch or a linker) needs to be propagated to be handled by a wrapper.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, should be done so far. All the apps use wrapGAppsHook
542b4a6
to
e07eab2
Compare
maintainers = [ maintainers.mkg20001 ]; | ||
|
||
knownVulnerabilities = [ | ||
"The runtime was extracted from Firefox 52, which EOL’d on September 5, 2018." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should go to the spdermonkey package. Though I am not sure if it will be possible to whitelist it then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The engine will only run code from cinnamon's source code or extensions, which are allowed to call commands anyways.
So we aren't lowering security by a lot by whitelisting it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What would whitelisting mean?
e56a660
to
fad6a28
Compare
Built with
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be good enough I suppose.
@@ -1,5 +1,6 @@ | |||
{ pkgs, lib }: | |||
|
|||
lib.makeScope pkgs.newScope (self: with self; { | |||
xapps = callPackage ./xapps {}; | |||
cjs = callPackage ./cjs { }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about something like:
cjs = callPackage ./cjs { }; | |
# This is insecure but we still want Hydra to build it. | |
unstafeCjsDoNotUse = callPackage ./cjs { }; | |
cjs = unstafeCjsDoNotUse // { | |
meta = unstafeCjsDoNotUse.meta // { | |
knownVulnerabilities = [ | |
"The SpiderMonkey runtime was extracted from Firefox 52, which EOL’d on September 5, 2018." | |
]; | |
}; | |
}; | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem is that there are things that depend on cjs. As far as I understand it, we'd have to do this for every package that uses cjs as well.
Motivation for this change
Needed for porting the cinnamon desktop environment.
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nix-review --run "nix-review wip"
./result/bin/
)nix path-info -S
before and after)Notify maintainers
cc @