-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
vscode: update to 1.96.4 #51909
vscode: update to 1.96.4 #51909
Conversation
|
FYI, about that failed x86_64-glibc build check, I just native-built the package on this arch and it turned out to be successful. |
The failure of the x64 build seems to stem from the process being killed after some time, not a failure with the build itself. |
I built this package myself as well and it built just fine. The automated builder just screws up, kills the process after an arbitrary delay and that prevents this package from being updated. |
I'm currently attempting to update this to 1.94.0, but they removed yarn in favor of using npm, which causes a lot of issues with the way we build things. |
There are still some issues, but I'm slowly getting there. The current version doesn't seem to run, as it expects are more recent node version. I guess we will need another electron update before this will work. @Johnnynator, any chance you could pick that up (since you did the last one)? I've checked it. The required API is |
Yes, I can do some Electron updates, kinda skipped too many of them. |
This version also includes a fix for an RCE CVE: https://msrc.microsoft.com/update-guide/vulnerability/CVE-2024-43601, so I'd like to get it to the users asap. |
As expected, have the same problem as we have with chromium right now, the v8 jit is broken on musl, need to either get that fixed or find a way to get jitless wasm (drumbrake) working in electron (electron 32 uses a chromium version that is one major version too old to have drumbrake included 😢 ) |
electron v33 was just released. I guess we could go up one version without too much changes. |
Nice, this might actually simplify things, since this should come with drumbrake, so we can workaround for now by getting that slower interpreter going. (Working slowly is better than not working at all). Still should probably do a git bisect of musl, to see if I can find the commit that fixes the jit (musl 1.1.x in our repo is broken, musl 1.2.x seems to work with the jit, and recompiling musl is way faster than even downloading different chromium versions). |
@Johnnynator: do you have a working build for x64-glibc so I can already prepare whatever changes are required for the electron update? |
Sorry, got a bit sidetracked and had some other things to do, some test builds are up: #52848 / https://void.johnnynator.dev/dev/electron33/ |
@Johnnynator I get an error that |
Yep, electron did remove the option to ship this tarball. There are headers in electron33-devel. (Kinda like Alpine is doing it) |
That's strange, I added electron33-devel but still got the same message. Maybe it is in a different path? Have to investigate later. Update: the headers seem already unpacked, no longer part of a tarball, so that's the issue here. Do we even need to run node-gyp in this case? Let's skip it for now. Update2: currently fighting with graceful-fs, which is required to fix the too many open file descriptors issue. I'll push an incomplete update in a few minutes. |
e12bf37
to
a62a1d9
Compare
Got it up and running. Neither node-gyp nor graceful-fs are required anymore. Now all we need is for electron33 to be merged and then, this is ready for review after a rebase. Update: the internal git extension seems to have issues. |
Debugging this shows the error "cannot find module mkdirp", which is strange, since the module is clearly present in the build. |
We still have the same error and I cannot figure out why this happens. The library is there, it is just not loaded... |
Bump. |
@shizonic do you have any idea? |
Would https://vscodium.com be an easier alternative to package? It seems more in line with void-linux principles. |
VSCodium could be considered a fork of Code-OSS even though they really want to emphasize that it isn't a fork (but it is; it's Code with patches on top to disable telemetry and the Microsoft extension marketplace), and Void Linux has a strict policy against forks, so I'm unsure whether this would be accepted. It ultimately is up to contributor discretion. |
I had a look at vscodium and it does not have any patches that would address our current issue if we still want to build with a local electron from sources. |
Yeah, I tried building it locally and a bunch of third party modules including but not limited to mkdirp are not present in Maybe it is because of removing Edit: I can confirm that is the issue: https://gist.github.com/oreo639/a2b2bb660cbaac71d30e65709de7d4c3 |
It seems there is already a fix within a later version of electron33, so I will try to build that, too. @Johnnynator, 33.3.2 fails with a build error. Could I bother you to update electron33 again? |
electron33 33.3.2 needs to be built with a specific chromium and npm release specified here: https://releases.electronjs.org/release/v33.3.2 I am currently building electron33 33.1.0 which is the first release including the patch, and the last electron33 series release that uses a non-extended stable verision of chromium. Extended stable versions of chromium don't have source tarballs available on the chromium-browser-official since they are primarily intended for commercial windows users. Different distros handle that differently, looking at the void-packages history, we just don't update to those versions, Arch Linux uses git, Alpine and OpenSUSE generate their own electron tarballs with chromium sources included (since electron does not provide official tarballs), and FreeBSD hosts the chromium tarballs as github releases but has it split into multiple files due to upload limits. (most distros handle it by not shipping electron :p) |
I did use the closest tarball in the past and included the upstream patches in void-packages to get it up the version expected by electron. But the last released chromium tarball is .129 :( |
Alright, I tested and can confirm that the crashing is resolved with electron33 33.1.0 although, as Johnnynator pointed out, I was mistaken as 33.2.0 is the last 33 series release using a non-extended stable chromium. |
+update to electron33
OK, after updating electron, this should work. |
👍 |
Testing the changes