Conversation
|
I will need to fix the update script to update surfer and firefox. |
The PR had not only had technical issues, but project management issues inherent to upstream as well. As such, drafting until a consensus amongst committees/security/buildMozillaMach folks can be reached. |
This comment was marked as spam.
This comment was marked as spam.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
Looking at Surfer's code, it seems to be quite simple just to patch out the Git usages, even being able to contribute most (if not all) of these fixes upstream. Can we try that?
There was a problem hiding this comment.
Yeah I could look at doing that.
There was a problem hiding this comment.
There are a lot of git uses though: https://github.com/search?q=repo%3Azen-browser%2Fsurfer+%27git%27&type=code
I have NodeJS experience and could sumit a patch to use isomorphic-git instead to remove the git dependency if that would be helpful. Never mind, it doesn't support some git commands that are needed.
There was a problem hiding this comment.
There are a lot of git uses though
Most of them are not relevant to us, so we can ignore them. On a cursory glance, I count three that we'd want to patch:
- https://github.com/zen-browser/surfer/blob/ecd6b650f0234402976dde7e775d42aec6568406/src/commands/patches/git-patch.ts#L16-L18
- https://github.com/zen-browser/surfer/blob/ecd6b650f0234402976dde7e775d42aec6568406/src/commands/build.ts#L34
- https://github.com/zen-browser/surfer/blob/ecd6b650f0234402976dde7e775d42aec6568406/src/commands/download/addon.ts#L223-L230 (if Zen even uses addons)
There was a problem hiding this comment.
I've just worked on two patches that patch the addon one and the changeset in build. But I am having a bit of trouble of patching the git patch one as that uses the patches from zen browser and applies it directly to the engine.
There was a problem hiding this comment.
Unless a. use IFD which is against nixpkgs or b. include every patch in diretory. it turns out that this is not IFD if it was it would ofborg would eval error + gh actions.
There was a problem hiding this comment.
This is still the case with updated code I am looking into solutions.
There was a problem hiding this comment.
And I am actually encouraging for people to add their work to my branch and to spit this up into seperate prs in my branch so this can get merged with the more work.
There was a problem hiding this comment.
@winterqt I do want to patch this however are you sure on what patches it applies to the source?
This comment was marked as off-topic.
This comment was marked as off-topic.
|
You would need to provide an example of a library in question, the error you run into, and why you think it's needed on Darwin. |
You could run |
This comment was marked as outdated.
This comment was marked as outdated.
5094a94 to
66629b1
Compare
72a2d56 to
8f31186
Compare
8f31186 to
10078c9
Compare
ah, yeah, I can see, there are also prefs and other stuff |
|
Sorry I have been inactive on this pr, I will be trying to get surfer issues that are mentioned in the comments here. Just reverted my source patching so this is at least working state. I am rethinking this a different way, to not over-complicate things while being a FOD (eg. patch all the git usages for now just to get this out). I know it has been a year, pr's like these take a long time, I am working my absolute best to get this out now thanks to a couple pr's out of the way, please be more patient.
Does zen-browser even have support for pgo? |
d3fdac8 to
0edb990
Compare
2ffa548 to
74dbe2c
Compare
| pname = "surfer-patched"; | ||
| version = "0-unstable-2026-01-25"; | ||
|
|
||
| src = fetchFromGitHub { |
There was a problem hiding this comment.
Does the update script update this?
There was a problem hiding this comment.
No, I have to change this. I just haven't had time. Also thank you for the reviews they are tiny nitpicks, I would like to remind of this guideline in the readme: https://github.com/NixOS/nixpkgs/blob/master/CONTRIBUTING.md#a-reviewer-requested-a-bunch-of-insubstantial-changes to avoid getting into trouble. I will get around to responding to your nitpicks on the weekend, as I have school.
UPDATE: I have responded to all of those nitpicks sept for the update script since I had spear time from school, I will do the update script this weekend since I have co-curricular activity's (school and non-school) and school.
There was a problem hiding this comment.
UPDATE: I didn't quite get my head around to fixing this (I have, but I haven't), But I am still working on this.
abd4b1c to
7592220
Compare
Co-authored-by: Matthew Penner <me@matthewp.io> Co-authored-by: Ruby Iris Juric <ruby@srxl.me> Co-authored-by: Yiyu Zhou <yiyuzhou19@gmail.com>
https://zen-browser.app/
NOTE: This package takes quite a lot of resources to build. It took me an hour maybe two on my machine (3200g), This is not a package you want to compile yourself if you can avoid it.
If anyone wants to be added as a maintainer to this package, please leave a comment and I will add you.
Followup from PR #347222 that got reverted because of this comment: #347222 (comment) I want to make sure this pr is okay to go on nixpkgs the right way instead of reverting.
Closes #327982
Tracking:
buildMozillaMachIf wanting to talk please talk in here if you want to contribute or preferably the matrix nix Mozilla chat.
I will not be accepting comments that aren't contributing to this pr directly and will be marked
off-topicby moderation it is wasting time on contributors who want to work on this pr.This work is semi-sponsored by DigitalBrewStudios.
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.