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

AppImage builder feature #996

Merged
merged 3 commits into from
Feb 1, 2023

Conversation

fang64
Copy link
Contributor

@fang64 fang64 commented Jan 1, 2023

I have created a PR to create AppImages using GitHub Actions Workflow.

The main modification I made was add a flag to BuildLinux.sh so it'll skip the memory check as it's not a problem to compile with less ram. That ram check could be removed but I've just simply added a flag to skip it.

I also added a check to see if you are attempting to run the AppImage build in a docker container so it'll patch the appimagetool so it'll run and use the extract and run option in that scenario. GH Action workflow isn't in a container but when testing with nektos/act it was needed to confirm the build was working correctly.

I've incorporated @zhaofengli 's fix Disable EGL for wxGLCanvas with my PR to get the appimage to work properly.

The artifacts work nearly perfectly and the AppImage builds work great.

@fang64 fang64 force-pushed the BS-AppImage-Builder-Feature branch from 9698163 to e2e9e9b Compare January 1, 2023 22:33
@lanewei120
Copy link
Collaborator

we have merged #1053

and it seems no need to disable egl for wxGLCanvas under Linux now

could you remove this commit and submit again?

@fang64
Copy link
Contributor Author

fang64 commented Jan 16, 2023

@lanewei120 I'll rebase this without that commit. I do need to add a dependency for Arch Linux users, but they can just install WebKit2gtk and it resolves it.

Adding this dependency is non-trivial, since I can't easily just assume it's dependencies would be satisfied enough. I think it's less grief at the moment to just let them install that as a external dependency.

@fang64 fang64 force-pushed the BS-AppImage-Builder-Feature branch 2 times, most recently from 80287e3 to b760cc6 Compare January 16, 2023 12:41
@fang64
Copy link
Contributor Author

fang64 commented Jan 16, 2023

It seems I'm getting build errors now that weren't happening previously I'm investigating now.

@lanewei120 I'm not sure if you've noticed this error but it doesn't work:
Boost.cmake mistake
PATCH_COMMAND git init && ${PATCH_CMD} ${CMAKE_CURRENT_LIST_DIR}/0001-Boost-fix.patch

The Result is this
cd /home/user/git/BambuStudio-SoftFever/deps/build/dep_Boost-prefix/src/dep_Boost && /usr/bin/cmake -E copy /home/user/git/BambuStudio-SoftFever/deps/build/boost-user-config.jam ./tools/build/src/tools/user-config.jam git init && /usr/bin/git apply --verbose --ignore-space-change --whitespace=fix /home/user/git/BambuStudio-SoftFever/deps/Boost/0001-Boost-fix.patch

I think the intent was
cd /home/user/git/BambuStudio-SoftFever/deps/build/dep_Boost-prefix/src/dep_Boost && /usr/bin/cmake -E copy /home/user/git/BambuStudio-SoftFever/deps/build/boost-user-config.jam ./tools/build/src/tools/user-config.jam && git init && /usr/bin/git apply --verbose --ignore-space-change --whitespace=fix /home/user/git/BambuStudio-SoftFever/deps/Boost/0001-Boost-fix.patch

I'm going to change it to this
PATCH_COMMAND && git init && ${PATCH_CMD} ${CMAKE_CURRENT_LIST_DIR}/0001-Boost-fix.patch

@fang64 fang64 force-pushed the BS-AppImage-Builder-Feature branch from b760cc6 to 5899b8b Compare January 16, 2023 13:40
@fang64
Copy link
Contributor Author

fang64 commented Jan 16, 2023

#1062 it seems this is the problem with compiles, if you need me to, I can remove that change.

@fang64 fang64 force-pushed the BS-AppImage-Builder-Feature branch from 5899b8b to c6345f8 Compare January 16, 2023 13:55
@lanewei120
Copy link
Collaborator

yes, this is a boost issue we submitted recently, and we will fix it soon

@fang64
Copy link
Contributor Author

fang64 commented Jan 17, 2023

@lanewei120 , To add I think it's worth just pulling @SoftFever's CICD (SoftFever/OrcaSlicer@96c861f) commits into this repo actually, I did look at cherry picking out those commits so we can at least get official builds generated on changes. Although it seems the build system has diverged quite a bit between that fork and the Official repository. The changes to the Mac OSX/Windows build process are pretty significant but that's probably out of scope for the PR.

I suspect Bambu Labs is already doing this internally in some way but at least PRs can be quickly verified here.

@lanewei120
Copy link
Collaborator

yes, this is a boost issue we submitted recently, and we will fix it soon

we have submitted the fix here: #1097

@lanewei120
Copy link
Collaborator

@lanewei120 , To add I think it's worth just pulling @SoftFever's CICD (SoftFever@96c861f) commits into this repo actually, I did look at cherry picking out those commits so we can at least get official builds generated on changes. Although it seems the build system has diverged quite a bit between that fork and the Official repository. The changes to the Mac OSX/Windows build process are pretty significant but that's probably out of scope for the PR.

I suspect Bambu Labs is already doing this internally in some way but at least PRs can be quickly verified here.

I will check that commit

currently we have an similiar flow to check the building and basic issues internally
I will check that commit and place a similiar mechanism on github

thanks

@fang64
Copy link
Contributor Author

fang64 commented Jan 24, 2023

I'm going to redo this AppImage build as using the community script seems to miss dependencies from the system. This works to a degree but fails on newer distributions like Arch/Fedora.

@lanewei120
Copy link
Collaborator

I'm going to redo this AppImage build as using the community script seems to miss dependencies from the system. This works to a degree but fails on newer distributions like Arch/Fedora.

are you using the newest codes on branch v1.4.2?

please use it which is a stable release of v1.4.2

@fang64
Copy link
Contributor Author

fang64 commented Jan 29, 2023

@lanewei120 I'll give it a go, but I already have seen better results using linuxdeployqt instead of the BuildLinux.sh script. To add I don't think the idea was to build separate AppImages for each distribution family.

It feels like a much more sane approach to dealing with the dependencies, as Sub Surface is using this approach to great success. for packaging AppImages.

@lanewei120
Copy link
Collaborator

the first commit of boost is also no need now

we have submited the fix 2 weeks ago here 698d51c

sorry for the inconvenience

@lanewei120
Copy link
Collaborator

@lanewei120 I'll give it a go, but I already have seen better results using linuxdeployqt instead of the BuildLinux.sh script. To add I don't think the idea was to build separate AppImages for each distribution family.

It feels like a much more sane approach to dealing with the dependencies, as Sub Surface is using this approach to great success. for packaging AppImages.

@fang64
we want to add this solution currently
could you refine the codes and submit it?

  1. remove the commit "Boost: merge the patch command into one"
  2. remove 'push' in actions

althrough linuxdeployqt maybe a better solution
we can use this one in current stage

BTW: I have verified it a test-folk
https://github.com/lanewei120/BambuStudio_test/actions/runs/4054677743

@fang64 fang64 force-pushed the BS-AppImage-Builder-Feature branch from c6345f8 to 917b865 Compare February 1, 2023 09:50
@lanewei120 lanewei120 merged commit fe5629f into bambulab:master Feb 1, 2023
c2h5oh pushed a commit to c2h5oh/BambuStudio that referenced this pull request May 31, 2023
…en (bambulab#996)

Fixed double-label for fan speed-up time, fixed label to include hyphen.
c2h5oh pushed a commit to c2h5oh/BambuStudio that referenced this pull request May 31, 2023
…en (bambulab#996)

Fixed double-label for fan speed-up time, fixed label to include hyphen.
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.

2 participants