flathub / nz.mega.MEGAsync

https://flathub.org/apps/details/nz.mega.MEGAsync
12 stars 9 forks source link

A working complete rewrite of flatpak manifest #94

Closed jasongodev closed 2 months ago

jasongodev commented 3 months ago

Good day every one, I created a new flatpak manifest that can build the version 5.x of MEGAsync.

This new build routine uses the latest recommendations of MEGA regarding the use of vcpkg as the dependency manager and cmake as the build program.

The previous compile steps using qmake can only compile version 4.x. Starting version 5, MEGA has strongly moved into the vcpkg ecosystem while the non-vcpkg build steps are cumbersome and most often do not compile. While the version 5.2 still compiled using the old steps using qmake, the resulting app did not worked at all. Most notable issue is the inability to select folders for syncing causing everything to just crash.

Now the challenge with using vcpkg inside flatpak-builder is that vcpkg downloads the sources from the internet during build. This is not possible with flatpak-builder as it is sandboxed and only allows downloading via the module sources declared in the flatpak manifest.

To solve this challenge, I transformed all the vcpkg dependencies as module sources in the manifest and created a series of code replacements in the vcpkg ports files so that vcpkg will not download the repos. It will now use the local git source or the extracted archives as set in the corresponding source path variables in the ports file. I placed comments in the manifest file to explain the process.

I retained some of the patch files that are relevant to the MEGA source. Some of the old patch files are not needed any more because they target build files specific to qmake which are not needed any more.

As a rule of thumb, all patches, fixes, and workarounds regarding the build process will be written in the manifest file as shell script. The rest of the patches that are specific to MEGA source code will be served as patch files.

May I also request to be one of the maintainers of this repository so I can better address issues moving forward.

All the best,

flathubbot commented 3 months ago

Started test build 142919

flathubbot commented 3 months ago

Build 142919 failed

flathubbot commented 3 months ago

Started test build 142921

flathubbot commented 3 months ago

Build 142921 failed

flathubbot commented 3 months ago

Started test build 142924

flathubbot commented 3 months ago

Build 142924 failed

flathubbot commented 3 months ago

Started test build 142943

flathubbot commented 3 months ago

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

flatpak install --user https://dl.flathub.org/build-repo/125996/nz.mega.MEGAsync.flatpakref
jasongodev commented 3 months ago

@tim77 Please merge this pull request.

flathubbot commented 3 months ago

Started test build 143446

flathubbot commented 3 months ago

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

flatpak install --user https://dl.flathub.org/build-repo/126503/nz.mega.MEGAsync.flatpakref
flathubbot commented 2 months ago

Started test build 143698

flathubbot commented 2 months ago

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

flatpak install --user https://dl.flathub.org/build-repo/126759/nz.mega.MEGAsync.flatpakref
tazihad commented 2 months ago

how can I test this build? it's not available Also, Did you fix the folder icon changes it's state during sync?

tim77 commented 2 months ago

@jasongodev thanks a lot. Sorry for delay. I don't even have time to review and test this properly now. Can someone test and confirm that everything works with this update? Also did upstream accept your patches in the end or not?

For co-maintaining this package please open ticket here https://github.com/flathub/flathub/issues

tazihad commented 2 months ago

@flathubbot please rebuild

tim77 commented 2 months ago

bot, build

flathubbot commented 2 months ago

Queued test build for nz.mega.MEGAsync.

flathubbot commented 2 months ago

Started test build 144571

flathubbot commented 2 months ago

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

flatpak install --user https://dl.flathub.org/build-repo/127646/nz.mega.MEGAsync.flatpakref
jasongodev commented 2 months ago

@jasongodev thanks a lot. Sorry for delay. I don't even have time to review and test this properly now. Can someone test and confirm that everything works with this update? Also did upstream accept your patches in the end or not?

For co-maintaining this package please open ticket here https://github.com/flathub/flathub/issues

Hi, the changes are mostly regarding how to make vcpkg work in flatpak build environment. There's no change in the upstream code and I even removed some patches that do not apply anymore in the current build framework. I also made the dependencies match the versions recommended by MEGA so we can have a flatpak build that is as close as the non-flatpak binaries.

jasongodev commented 2 months ago

how can I test this build? it's not available Also, Did you fix the folder icon changes it's state during sync?

@tazihad yes it's fixed. You can test my PR using my repo for the mean time: https://github.com/jasongodev/nz.mega.MEGAsync/tree/master

Also note that as we speak I will be pushing some improvements regarding the static library recognition during vcpkg build inside flatpak. Still, my fork repo will have a working build.

bbhtt commented 2 months ago

How many files are you patching with sd ? It's better to directly use patches instead of those large regexes, that may end in a disaster.

A patch will fail if it does not apply

bbhtt commented 2 months ago

Also why is this bundling critical dependencies like xz and openssl? These are already in runtime.

jasongodev commented 2 months ago

Also why is this bundling critical dependencies like xz and openssl? These are already in runtime.

I'm currently writing a new manifest that does not bundle libraries already in flatpak runtime. I will commit soon.

jasongodev commented 2 months ago

How many files are you patching with sd ? It's better to directly use patches instead of those large regexes, that may end in a disaster.

A patch will fail if it does not apply

@bbhtt it's easier to code and test this way. For the time being I prefer this way than making patch files.

bbhtt commented 2 months ago

Can you squash the commits ? Reviewing 170 commits is not possible.

jasongodev commented 2 months ago

Can you squash the commits ? Reviewing 170 commits is not possible.

@bbhtt yes will do this for the new commit I will do later today. The one I'm working right now will use the static libraries from flatpak runtime if available.

hfiguiere commented 2 months ago

@bbhtt it's easier to code and test this way. For the time being I prefer this way than making patch files.

It's harder for anyone to understand and review. WTF is sd anyway? As you may have noticed maintainers sometime go MIA and then the next one need to understand what's going on. And using obscure build command with obscure tools doesn't help with that.

hfiguiere commented 2 months ago

Hi, the changes are mostly regarding how to make vcpkg work in flatpak build environment.

WHat would be helpful here is a tool that take the vcpkg input and output a manifest from it. Something generic. It seems that the intersection of people who use vcpkg and people who can do that has been an empty lately...

jasongodev commented 2 months ago

@bbhtt it's easier to code and test this way. For the time being I prefer this way than making patch files.

It's harder for anyone to understand and review. WTF is sd anyway? As you may have noticed maintainers sometime go MIA and then the next one need to understand what's going on. And using obscure build command with obscure tools doesn't help with that.

The use of sd as a regex patcher has something to do with the lots of portfiles in vcpkg that needs to be patched so it prevents it from downloading the sources during build. Yes we can use patch files but it will take a lot of patch files to maintain. A generic pattern for matching and replacing serves a practical purpose.

If it pleases you for me to transform all these practical regex transforms into regular patch files, I will gladly comply. But for now I'm still figuring and ironing out all the patches needed so vcpkg will work in flatpak build, hence the practicality of using sd for the mean time.

jasongodev commented 2 months ago

Hi, the changes are mostly regarding how to make vcpkg work in flatpak build environment.

WHat would be helpful here is a tool that take the vcpkg input and output a manifest from it. Something generic. It seems that the intersection of people who use vcpkg and people who can do that has been an empty lately...

Yes that mythical tool will be so welcomed. But that does not exist yet. The fact is that it might be the first time that a vcpkg-based code base will be published in flatpak. It's a working proof of concept that I'm offering here.

In the long run, someone will figure out to add features in vcpkg to have a port overlay or some other mechanism to use a local git or local archive extract, without the need for patching the portfiles. The mechanism will be similar to what I did. But for now it does not exists in vcpkg and so we are left to do what we have to do.

hfiguiere commented 2 months ago

The fact is that it might be the first time that a vcpkg-based code base will be published in flatpak.

No it is not.

Just that's the first time such a complicated and unmaintainable solution is used.

hfiguiere commented 2 months ago

The use of sd as a regex patcher has something to do with the lots of portfiles in vcpkg

The use of sd, a tool no one here had heard about is by itself part of the issue. Maintainers / reviewers at large are the one that end up having to deal with this making their limited time wasted.

And each time someone say "I implemnent this hack instead of fixing the root cause" and one more time where this will never be fixed.

All the drawbacks without any of the advantages.

jasongodev commented 2 months ago

The fact is that it might be the first time that a vcpkg-based code base will be published in flatpak.

No it is not.

Just that's the first time such a complicated and unmaintainable solution is used.

Show me a flatpak app that is vcpkg-based. Maybe we can learn a technique or two from them.

jasongodev commented 2 months ago

The use of sd as a regex patcher has something to do with the lots of portfiles in vcpkg

The use of sd, a tool no one here had heard about is by itself part of the issue. Maintainers / reviewers at large are the one that end up having to deal with this making their limited time wasted.

And each time someone say "I implemnent this hack instead of fixing the root cause" and one more time where this will never be fixed.

All the drawbacks without any of the advantages.

Fixing the root cause? I know. Making vcpkg work natively inside a flatpak build environment. Can it be done? Yes, by pushing code in vcpkg upstream so they support flatpak build natively. It's a hell lot of work but it can be done. But for the mean time we are fixing MEGAsync build routine for flatpak.

If you believe we can't have the fix for MEGA first then by all means let's do it your way and wait another couple of months while MEGA is in unusable state. Or maybe we let MEGA upstream just do this flatpak thing and not waste time.

jasongodev commented 2 months ago

@tim77 @hfiguiere @bbhtt @tazihad I will close this first and will make a new pull request incorporating the following remarks that have been made by the reviewers of this pull request:

Thank you for all the suggestions.

sudoshindo commented 2 months ago

Where's the new PR?

jasongodev commented 2 months ago

Where's the new PR?

Next week.

Victor239 commented 2 months ago

Hello, any update on this?

loukamb commented 2 months ago

Pinging this. MEGAsync is critical to my work environment and I am happy to see that someone else is ready to take up the mantle for this flatpak. It’s unfortunate that working solutions are discarded so quickly because of what seems to simply be unfamiliarity, but at least the face my colleague made when I said I can’t pull down project files because of a “developer scared of regex” compensated for it.

hfiguiere commented 2 months ago

The PR was closed by the OP.

Instead of being insulting to the reviewers (I know regexp, thank you very much, that's why I believe it's wrong to use here), you could submit an update. Make sure to follow the review comments here.