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

Add several dependencies for QtWebEngine #178

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Dennisbonke
Copy link
Member

@Dennisbonke Dennisbonke commented Apr 27, 2022

This PR aims to add all the needed dependencies for QtWebEngine to compile. While QtWebEngine doesn't compile fully with this, it does allow Chromium to build pretty decently, which is the next target.

The following ports have been added:

  • pciutils;
  • qtwebsockets;
  • qtwebchannel.

Blocked on managarm/mlibc#416
Blocked on managarm/mlibc#418
Blocked on #211

Signed-off-by: Dennis Bonke <[email protected]>
@Dennisbonke Dennisbonke force-pushed the qtwebkit-deps branch 2 times, most recently from c2dc89e to 81fc2c6 Compare April 28, 2022 09:38
@Dennisbonke Dennisbonke force-pushed the qtwebkit-deps branch 2 times, most recently from 3264bd5 to ac93e77 Compare May 17, 2022 23:07
@Dennisbonke Dennisbonke marked this pull request as ready for review May 17, 2022 23:08
@Dennisbonke Dennisbonke changed the title Add several dependencies for QtWebkit Add several dependencies for QtWeEngine May 17, 2022
@Dennisbonke Dennisbonke changed the title Add several dependencies for QtWeEngine Add several dependencies for QtWebEngine Aug 7, 2022
@Dennisbonke Dennisbonke requested a review from avdgrinten August 7, 2022 12:09
Comment on lines +17 to +22
/// Choose a default value for the -j (parallelism) flag.
int GuessParallelism() {
+ int j = 0;
+ char* jobs = getenv( "NINJAJOBS" );
+ if ( jobs != NULL ) j = atoi( jobs );
+ if ( j > 0 ) return j;
Copy link
Member

Choose a reason for hiding this comment

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

Why do we patch ninja instead of just using the -j flag? Patching ninja doesn't sound really appealing to me.

Copy link
Member Author

Choose a reason for hiding this comment

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

If ninja is invoked by a different build system or a script (for example, qtwebengine, which builds with cmake but then builds chromium with ninja), an user can't pass the -j flag. With this patch, we can also use the NINJAJOBS environment variable. The patch was sourced from BLFS.

Copy link
Member

Choose a reason for hiding this comment

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

When using the build subcommand of cmake, you can pass arguments to make/ninja like so: cmake build -- -jN

Copy link
Member Author

Choose a reason for hiding this comment

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

Interesting, I didn't know that. In that case it's possible to drop the patch, tho I don't mind keeping it either.

Comment on lines 19 to 20
-#if HAVE_SCHED_GETAFFINITY && defined(CPU_COUNT)
+#if HAVE_SCHED_GETAFFINITY && defined(CPU_COUNT) && !defined(__managarm__)
Copy link
Member

Choose a reason for hiding this comment

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

Why is this done?

Comment on lines +22 to +23
-SYMBOL_VERSION(pci_filter_init_v38, pci_filter_init@LIBPCI_3.3);
+//SYMBOL_VERSION(pci_filter_init_v38, pci_filter_init@LIBPCI_3.3);
Copy link
Member

Choose a reason for hiding this comment

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

Why is this done?

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 got strange linker errors related to this, so I removed it. If requested, we could investigate it and fix those instead.

Comment on lines +91 to +109
+#ifdef __managarm__
+static void outb(unsigned char value, unsigned short int port) {}
+
+static void outw(unsigned short int value, unsigned short int port) {}
+
+static void outl(unsigned int value, unsigned short int port) {}
+
+static unsigned char inb(unsigned short int port) {
+ return 0;
+}
+
+static unsigned short int inw(unsigned short int port) {
+ return 0;
+}
+
+static unsigned int inl(unsigned short int port) {
+ return 0;
+}
+#endif
Copy link
Member

Choose a reason for hiding this comment

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

This does not look correct. We should not silently return wrong values (?).

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed we shouldn't, but at the time of creating this patch the libc wrapper functions didn't exist yet and I was too lazy to patch them in properly. With the correct mlibc PR merged, we might be able to drop this part of the patch completely.

Comment on lines +131 to +132
-#ifdef PCI_OS_LINUX
+#if defined PCI_OS_LINUX && !defined __managarm__
Copy link
Member

Choose a reason for hiding this comment

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

Why is this done?

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 honestly don't remember anymore, we might be able to just drop this part.

@@ -0,0 +1,67 @@
From 0c716d435abe65250100c2caea0e5126ac4e14bd Mon Sep 17 00:00:00 2001
Copy link
Member

Choose a reason for hiding this comment

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

This file does not match our usual patch naming convention.

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 was taken as-is from gentoo, so I kept the name intact as well.

@@ -0,0 +1,35 @@
From 516fdcca6606502e2d562d20c01b225c8d066739 Mon Sep 17 00:00:00 2001
Copy link
Member

Choose a reason for hiding this comment

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

This file does not match our usual patch naming convention.

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 was taken as-is from gentoo, so I kept the name intact as well.

Copy link
Member

Choose a reason for hiding this comment

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

The issue is that xbstrap sorts patches by their file name, so if we're missing the 0001- prefix, they'll be applied in "random" orders.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants