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.

Sign up for GitHub

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

Jump to bottom

[FEATURE] Create linux AppImage for building CCExtractor #1592

Merged
merged 4 commits into from
Mar 3, 2024
Merged

[FEATURE] Create linux AppImage for building CCExtractor #1592

merged 4 commits into from
Mar 3, 2024

Conversation

IshanGrover2004
Copy link
Contributor

@IshanGrover2004 IshanGrover2004 commented Jan 27, 2024 •
edited
Loading

In raising this pull request, I confirm the following:

  • I have read and understood the contributors guide.
  • I have checked that another pull request for this purpose does not exist.
  • I have considered, and confirmed that this submission will be valuable to others.
  • I accept that this submission may not be used, and the pull request closed at the will of the maintainer.
  • I give this submission freely, and claim no ownership to its content.
  • I have mentioned this change in the changelog.

My familiarity with the project is as follows (check one):

  • I have never used CCExtractor.
  • I have used CCExtractor just a couple of times.
  • I absolutely love CCExtractor, but have not contributed previously.
  • I am an active contributor to CCExtractor.

Addresses Issue #1348

What I did (Current state):

  • Replace non-working link
  • Replace not working or irrelevant commands
  • All library which are required in gcc command are installed correctly
  • To compile the ccextractor with all static libraries, we need to have correct configure.ac and Makefile.am file (currently its giving error, maybe I am doing anything wrong here)

Goal for now:

  • Compiling and make ccextracter binary file(which is required while making AppImage)

Code breaks at:

  • When it clones ccextracter and try to run make command(here).

@IshanGrover2004
Copy link
Contributor Author

IshanGrover2004 commented Jan 27, 2024 •
edited
Loading

I am using these commands(link):

autoreconf --install
./configure --without-rust
make ENABLE_OCR=yes

which gives error:

In file included from ../src/lib_ccx/params.c:13:
../src/lib_ccx/compile_info.h:7:10: fatal error: compile_info_real.h: No such file or directory
    7 | #include "compile_info_real.h"
      |          ^~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make: *** [Makefile:3073: ../src/lib_ccx/ccextractor-params.o] Error 1

(My guess is: the confiure.ac & Makefile.am needs to be fixed)
I wanted to confirm, am I doing this correct?

@IshanGrover2004 IshanGrover2004 mentioned this pull request Jan 30, 2024
Open
@prateekmedia
Copy link
Member

@IshanGrover2004 is it working yet? Or are you still stuck with the error

@IshanGrover2004
Copy link
Contributor Author

Is it working yet? Or are you still stuck with the error

Stuck with the error

@prateekmedia
Copy link
Member

prateekmedia commented Jan 30, 2024 •
edited
Loading

Maybe alpine and docker are causing this, are you sure this is the right way to build AppImage? There's also appimagetool used for building it.

@canihavesomecoffee
Copy link
Member

canihavesomecoffee commented Jan 30, 2024 •
edited
Loading

You're not running the pre-build.sh script that generates that header file ;)

(or using cmake; 4fe82ab)

@IshanGrover2004
Copy link
Contributor Author

Maybe alpine and docker are causing this, are you sure this is the right way to build AppImage? There's also appimagetool used for building it.

Currently my goal is to get ccextractor binary file which should statically linked then i will attach that binary file in AppImage
For more info read PR discription.

@IshanGrover2004
Copy link
Contributor Author

IshanGrover2004 commented Jan 30, 2024 •
edited
Loading

You're not running the pre-build.sh script that generates that header file ;)

(or using cmake; 4fe82ab)

Okk, I'll try this.
Can you have a look here, is this the right way or should i have to change to some other way

@IshanGrover2004
Copy link
Contributor Author

You're not running the pre-build.sh script that generates that header file ;)

Yeah that fixed it (I used autogen.sh)

From past few days, I tried many thing to fix this script and I am just stuck with the

/usr/lib/gcc/x86_64-alpine-linux-musl/13.2.1/../../../../x86_64-alpine-linux-musl/bin/ld: cannot find -lgpac: No such file or directory
collect2: error: ld returned 1 exit status
make: *** [Makefile:1811: ccextractor] Error 1

I did build libgpac from source and i able to run gcc temp.c -lgpac and i can see the binaries installed(at /usr/local/lib/ & ...)

@prateekmedia
Copy link
Member

@IshanGrover2004 have you installed libgpac on your system or you are installing it in the alpine linux under this script?

@IshanGrover2004
Copy link
Contributor Author

IshanGrover2004 commented Feb 4, 2024 •
edited
Loading

@IshanGrover2004 have you installed libgpac on your system or you are installing it in the alpine linux under this script?

I am installing in alpine linux in docker

It is failing in mine(Arch) as well

/usr/bin/ld: cannot find -lgpac: No such file or directory
collect2: error: ld returned 1 exit status
make: *** [Makefile:1811: ccextractor] Error 1

@prateekmedia
Copy link
Member

@IshanGrover2004 The alpine and docker method may have some outdated steps, if you really want to build AppImage I would suggest to use this package:
https://github.com/AppImageCrafters/appimage-builder

This will help you write for ubuntu or other distro and write recipe and generate AppImage.

@IshanGrover2004
Copy link
Contributor Author

IshanGrover2004 commented Feb 4, 2024 •
edited
Loading

@IshanGrover2004 The alpine and docker method may have some outdated steps

That i got to know at very 1st day when i saw this script, so I was also finding some alternative method. Thank u for finding

If you really want to build AppImage I would suggest to use this package:
https://github.com/AppImageCrafters/appimage-builder

This will help you write for ubuntu or other distro and write recipe and generate AppImage.

I'll have a look and try as well

@IshanGrover2004
Copy link
Contributor Author

if you really want to build AppImage I would suggest to use this package: https://github.com/AppImageCrafters/appimage-builder

@prateekmedia Still we want to have first ccextractor executable bin file which should be statically linked
And there is where problem occurs

@prateekmedia
Copy link
Member

I am able to build already, I think you should refer to this:
https://github.com/CCExtractor/ccextractor/wiki/Installation#linux

and

https://github.com/CCExtractor/ccextractor/blob/master/docs/COMPILATION.MD

@IshanGrover2004
Copy link
Contributor Author

I am able to build already, I think you should refer to this: https://github.com/CCExtractor/ccextractor/wiki/Installation#linux

and

https://github.com/CCExtractor/ccextractor/blob/master/docs/COMPILATION.MD

I am already aware of these resources, this will result a binary which is dynamic linked(you can check by ldd <binary>)
but we need to perform static build & that is what I am trying to do by

git clone https://github.com/CCExtractor/ccextractor
cd ccextractor/linux/
perl -i -pe 's/O3 /O3 -static /' Makefile.am
set +e
./autogen.sh
./configure --without-rust
make ENABLE_OCR=yes

My thought process is that if somehow I can have some files(lets say a header file or obj files) which i can attach with gcc compiler to hand compile with all static(.a) binaries needed like this

Lets take a example to explain my approach:
I will install all necessary library statically(Eg. lib1.a, lib2.a) and I have some program(main.c, etc)
So to compile I can just do gcc main.c [more program files] lib1.a lib2.a -o out

That's why I add obj/* here by thinking if i can get obj files from C source files by make
I may be very wrong so correct me @prateekmedia . And if you understand my approach so now you could tell how can i get objs/* kinda thing

@prateekmedia
Copy link
Member

You should not care about dynamic or static, at the end you have to bundle the libraries with .so related to build in the appimage

@IshanGrover2004
Copy link
Contributor Author

You should not care about dynamic or static, at the end you have to bundle the libraries with .so related to build in the appimage

So do I need to make just AppImage(and doesn't care about static or dynamic)
Because i thought, maybe some people really care about static or dynamic build(that's why build-static exist)

@prateekmedia
Copy link
Member

prateekmedia commented Feb 4, 2024 •
edited
Loading

If you are bundling every library then nothing matters, although do remember if some system have minimum library version requirement like GLIBC 2.32 or more, then you just have to cater that.

@IshanGrover2004
Copy link
Contributor Author

So should I change this PR to actually build AppImage for ccextractor (instead of build-static)

@prateekmedia
Copy link
Member

You should just build and bundle libraries to AppImage and test in different ubuntu versions to check backward compatibility.

@IshanGrover2004 IshanGrover2004 changed the title [FEATURE] Create linux AppImage for build-static [FEATURE] Create linux AppImage for building CCExtractor Feb 5, 2024
@IshanGrover2004
Copy link
Contributor Author

IshanGrover2004 commented Feb 5, 2024 •
edited
Loading

You should just build and bundle libraries to AppImage and test in different ubuntu versions to check backward compatibility.

Is this the exact thing for what you want @prateekmedia ?

@prateekmedia
Copy link
Member

You should just build and bundle libraries to AppImage and test in different ubuntu versions to check backward compatibility.

ig this is the exactly what you want, Is it @prateekmedia ?

Yeah, something like that.

@IshanGrover2004 IshanGrover2004 force-pushed the 1348-create-linux-appimage branch from 1e66959 to d408234 Compare February 6, 2024 19:52
@IshanGrover2004
Copy link
Contributor Author

IshanGrover2004 commented Feb 10, 2024 •
edited
Loading

After a lot struggle with different AppImage tools, I successfully made an AppImage which is working.
@prateekmedia I will first revert the all the commits above for build_static and delete build_static.sh and push the current script for building AppImage which is working correctly.

Edit: All done

All I need right now is a photo of CCExtractor logo for AppImage icon of any of these resolution(8x8, 16x16, 20x20, 22x22, 24x24, 28x28, 32x32, 36x36, 42x42, 48x48, 64x64, 72x72, 96x96, 128x128, 160x160, 192x192, 256x256, 384x384, 480x480, 512x512) in png extension which is directly accessible by wget(Or maybe do upload it in website repo)

Edit: Got the photo and opened a PR for uploading in Website repo

@IshanGrover2004 IshanGrover2004 force-pushed the 1348-create-linux-appimage branch from d408234 to f6d3463 Compare February 10, 2024 18:57
@IshanGrover2004 IshanGrover2004 mentioned this pull request Feb 11, 2024
Merged
@IshanGrover2004
Copy link
Contributor Author

This PR is ready to review and get merged

@prateekmedia
Copy link
Member

@IshanGrover2004 have you tested the generated AppImage in your system and some other system without any dependency installed like in fresh Ubuntu 20.04?

@IshanGrover2004
Copy link
Contributor Author

IshanGrover2004 commented Feb 11, 2024 •
edited
Loading

have you tested the generated AppImage in your system and some other system without any dependency installed like in fresh Ubuntu 20.04?

This script file result AppImage when your system has necessary lib installed(able to run cmake ../src; make; in ccextrcator repo)
For making it system independent we have to install every library manually and setup them in AppDir which will be ig not good idea

@prateekmedia
Copy link
Member

prateekmedia commented Feb 11, 2024 •
edited
Loading

@IshanGrover2004 We need to supply tessdata within AppImage, that was the purpose of this:
#1348 (comment)
#1348 (comment)

@prateekmedia
Copy link
Member

prateekmedia commented Feb 11, 2024 •
edited
Loading

Packaging libraries within the AppImage is needed to make it really replacement of static i.e. system independent as static libraries are packaged already.

@IshanGrover2004
Copy link
Contributor Author

Okk let me try that as well

@IshanGrover2004 IshanGrover2004 force-pushed the 1348-create-linux-appimage branch from 1ba7139 to f3145e3 Compare February 19, 2024 10:43
@ccextractor-bot
Copy link
Collaborator

CCExtractor CI platform finished running the test files on linux. Below is a summary of the test results, when compared to test for commit 2ada36d...:

Report Name Tests Passed
Broken 12/13
CEA-708 14/14
DVB 4/7
DVD 3/3
DVR-MS 2/2
General 16/27
Hauppage 1/3
MP4 2/3
NoCC 10/10
Options 82/86
Teletext 21/21
WTV 2/13
XDS 27/34

All tests passing on the master branch were passed completely.

NOTE: The following tests have been failing on the master branch as well as the PR:

Congratulations: Merging this PR would fix the following tests:


Check the result page for more info.

canihavesomecoffee
canihavesomecoffee approved these changes Mar 3, 2024
View reviewed changes
Copy link
Member

@canihavesomecoffee canihavesomecoffee left a comment

Choose a reason for hiding this comment

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

lgtm, it builds on my machine; seems to work with a random sample for test purposes.

We should possibly update and include this in a release build.

@cfsmp3 cfsmp3 merged commit f08febf into CCExtractor:master Mar 3, 2024
8 checks passed
@IshanGrover2004
Copy link
Contributor Author

@canihavesomecoffee This currently build AppImage successfully but it needs improvisation, So Whenever I'll get time to update I will and once achieved open PR

@IshanGrover2004 IshanGrover2004 deleted the 1348-create-linux-appimage branch May 11, 2024 10:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@canihavesomecoffee canihavesomecoffee canihavesomecoffee approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@IshanGrover2004 @prateekmedia @canihavesomecoffee @ccextractor-bot @cfsmp3