flathub / com.bambulab.BambuStudio

https://flathub.org/apps/details/com.bambulab.BambuStudio
14 stars 3 forks source link

Cleanup patches are as consistent as possible with the bambu studio master branch #25

Closed MackBambu closed 5 months ago

MackBambu commented 5 months ago

Bambu master branch receives partial patches

flathubbot commented 5 months ago

Started test build 127557

MackBambu commented 5 months ago

I created two repositories to receive and test if the patches work.

flathubbot commented 5 months ago

Build 127557 failed

hadess commented 5 months ago

I created two repositories to receive and test if the patches work.

I don't understand what this means. If you're just doing build testing, it would be great if you could run those tests locally before sending merge requests that are unmergeable.

MackBambu commented 5 months ago

I don't understand what this means. If you're just doing build testing, it would be great if you could run those tests locally before sending merge requests that are unmergeable.

Don't worry, I'm in no hurry to merge code. My current job is to merge some patches into the bambu master branch, but it needs a lot of testing before that. This PR still needs a lot of changes with your help.

MackBambu commented 5 months ago

@hadess Is there any flag to detect the flatpak compilation environment in cmake? I'm trying to merge use-sysdeps.patch into Bambu studio so we can drop this.

hadess commented 5 months ago

@hadess Is there any flag to detect the flatpak compilation environment in cmake? I'm trying to merge use-sysdeps.patch into Bambu studio so we can drop this.

You can set any environment variable in the Flatpak manifest, look for env in the flatpak-manifest man page.

I think a better way to do this would be to add specific CMake options to allow sys dependencies for all the internal dependencies we disable in that patch, like wxwidgets allows:

  - name: wxwidgets
    buildsystem: cmake-ninja
    config-opts:
      - -DwxBUILD_PRECOMP=ON
      - -DwxBUILD_TOOLKIT=gtk3
      - -DwxBUILD_DEBUG_LEVEL=0
      - -DwxBUILD_SAMPLES=OFF
      - -DwxBUILD_SHARED=OFF
      - -DwxUSE_MEDIACTRL=ON
      - -DwxUSE_DETECT_SM=OFF
      - -DwxUSE_UNICODE=ON
      - -DwxUSE_PRIVATE_FONTS=1
      - -DwxUSE_OPENGL=ON
      - -DwxUSE_WEBREQUEST=ON
      - -DwxUSE_WEBVIEW=ON
      - -DwxUSE_REGEX=sys
      - -DwxUSE_LIBSDL=OFF
      - -DwxUSE_XTEST=OFF
      - -DwxUSE_STC=OFF
      - -DwxUSE_AUI=ON
      - -DwxUSE_LIBPNG=sys
      - -DwxUSE_ZLIB=sys
      - -DwxUSE_LIBJPEG=sys
      - -DwxUSE_LIBTIFF=sys
      - -DwxUSE_NANOSVG=OFF
      - -DwxUSE_EXPAT=sys

If we had this for libpng, libjpeg, tiff, boost, etc. we could drop the patch and set some config options instead.

MackBambu commented 5 months ago

If we had this for libpng, libjpeg, tiff, boost, etc. we could drop the patch and set some config options instead.

Look at this. https://github.com/MackBambu/BambuStudio/commit/3c4a307a64fb621cf830d5941ec2fd006eb7d088

hadess commented 5 months ago

Look at this. MackBambu/BambuStudio@3c4a307

Much better. Let me know when you've submitted this in an MR, and I'll review it.

flathubbot commented 5 months ago

Started test build 127720

flathubbot commented 5 months ago

Build 127720 failed

flathubbot commented 5 months ago

Started test build 127725

hadess commented 5 months ago

It would be great if you could try not to use this shared resource as a build server. Please build your changes locally (you can even reference local file:/// paths for git, so that you don't have to push your changes to the server).

The "close GLFW build TEST and EXAMPLES" commit you pushed is already in this repository under the subject "Disable docs building in glfw", you'll need to rebase your branch to get it.

MackBambu commented 5 months ago

It would be great if you could try not to use this shared resource as a build server. Please build your changes locally (you can even reference local file:/// paths for git, so that you don't have to push your changes to the server).

The "close GLFW build TEST and EXAMPLES" commit you pushed is already in this repository under the subject "Disable docs building in glfw", you'll need to rebase your branch to get it.

I actually built everything fine locally by running local-build.sh. :(

图片

flathubbot commented 5 months ago

Build 127725 failed

MackBambu commented 5 months ago

Why are the results different between server and local builds?

flathubbot commented 5 months ago

Started test build 127729

flathubbot commented 5 months ago

Build 127729 successful To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/110714/com.bambulab.BambuStudio.flatpakref
flathubbot commented 5 months ago

Started test build 128882

MackBambu commented 5 months ago

@hadess I think it's ready. please reviewer Here are the wxWidgets after merging the patches: https://github.com/bambulab/wxWidgets/commits/master/ Here's the studio commit for the merged patch: https://github.com/bambulab/BambuStudio/commit/c8378296fa42836408dceb877452b7d4aa32c1f6 https://github.com/bambulab/BambuStudio/commit/a181250668fae3110e329c0dd289f1e71dd1bb45

flathubbot commented 5 months ago

Build 128882 successful To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/111869/com.bambulab.BambuStudio.flatpakref
flathubbot commented 5 months ago

Started test build 128890

flathubbot commented 5 months ago

Build 128890 successful To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/111877/com.bambulab.BambuStudio.flatpakref
hadess commented 5 months ago

This isn't in a reviewable state. Please rework your changes to include one justified change per commit message.

I'm not that interested in relying on vendor forks of the upstream libraries, as I'd rather we shipped a small number of patches that we could reduce over time.

The right thing to do, in wxWidgets, is to make BambuStudio use the latest versions of wXwidgets, and fix the bugs that were found upstream.

hadess commented 5 months ago

Also note that we only ship releases of BambuStudio, so we wouldn't be able to merge this as it would require shipping the current development version.

MackBambu commented 5 months ago

The right thing to do, in wxWidgets, is to make BambuStudio use the latest versions of wXwidgets, and fix the bugs that were found upstream.

wxWidgets won't be upgraded anytime soon, this could be a slow process. You can check out similar projects like Prusa Studio, Orca Slicer, which is common practice. https://github.com/prusa3d/PrusaSlicer/blob/master/deps/%2BwxWidgets/wxWidgets.cmake#L31 https://github.com/SoftFever/OrcaSlicer/blob/main/deps/wxWidgets/wxWidgets.cmake#L27

hadess commented 5 months ago

wxWidgets won't be upgraded anytime soon, this could be a slow process. You can check out similar projects like Prusa Studio, Orca Slicer, which is common practice.

I'm not interested in relying on a vendored dependency in the Flatpak, I'd rather ship the upstream version with patches, it's really not that big a deal for our packaging.

MackBambu commented 5 months ago

We want the code to be as consistent as possible across platforms

MackBambu commented 5 months ago

The master branch of Bambu Studio has switched to relying on the forked version of wxWidget, so I don't see what's wrong with that? Shouldn't the dependency sources be consistent?

MackBambu commented 5 months ago

Also note that we only ship releases of BambuStudio, so we wouldn't be able to merge this as it would require shipping the current development version.

  1. Release 1.9.2
  2. Merge my PR
  3. Wait release 1.9.3
hadess commented 5 months ago

Please rework the commits as requested before we can discuss individual changes.

MackBambu commented 5 months ago

Please rework the commits as requested before we can discuss individual changes.

ok, I'll reorganise the commits when we release 1.9.3. As for the wxWidgets source, I think you have to think about it a bit more, we'll keep changing wxWidgets in the future and your patch file will get bigger and bigger.

hadess commented 5 months ago

Closing in favour of https://github.com/flathub/com.bambulab.BambuStudio/pull/28