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

rewrite external projects support #1847

Merged
merged 16 commits into from
Jul 15, 2024
Merged

Conversation

benoit-pierre
Copy link
Contributor

@benoit-pierre benoit-pierre commented Jul 13, 2024

Don't use CMake's ExternalProject support, this allow for consolidating some steps:

  • download: clone / download to thirdparty/project/build/source / thirdparty/project/build/downloads
  • prepare: (re-)create source directory, apply patches (overlay, files, command).
  • configure: (re-)create build directory (or clean up source directory when not building out-of-tree), configure project
  • build: self-explanatory
  • install: self-explanatory

Merging the extract / checkout creation with applying patches into one
prepare step ensure the later are always applied starting from a pristine
source tree: interrupting the step will not result in a broken (partially
patched) tree.

Steps are triggered on the following:

  • download: parameters changed
  • prepare: download step ran, patch overlay contents changed, patch list and/or contents changed, patch command changed
  • configure: prepare step ran, command line changed, koreader/toolchain file contents changed, direct dependency change (e.g. a zlib's version change — archive MD5 or Git revision — trigger a re-build of libpng)
  • build: configure step ran, command line changed, source file timestamp changed, direct dependency install step ran
  • install: build step ran, command line changed

Note:

  • make generator support has been dropped
  • configure depends globs are not used because they break dry-run, so it's possible that an addition to a patch overlay (new file) won't trigger the prepare step if there are no other changes that would trigger a CMake reconfiguration (like a change to CMakeLists.txt)

This change is Reviewable

@poire-z
Copy link
Contributor

poire-z commented Jul 13, 2024

(Impressive, this amount of black magic that I don't understand :) If most/all of this is to answer my recent complains about building, thank you :) )

Copy link
Member

@Frenzie Frenzie left a comment

Choose a reason for hiding this comment

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

Sounds good and looks good. 👍

No point in using it with make>=4.4.
To always run cmake (handy when hacking on the build system).
Dump build timings for the last ninja invocation.
Don't use CMake's `ExternalProject` support, this allow for consolidating some steps:
- download: clone / download to `thirdparty/project/build/source` / `thirdparty/project/build/downloads`
- prepare: (re-)create source directory, apply patches (overlay, files, command).
- configure: (re-)create build directory (or clean up source directory when not building out-of-tree), configure project
- build: self-explanatory
- install: self-explanatory

Merging the extract / checkout creation with applying patches into one
prepare step ensure the later are always applied starting from a pristine
source tree: interrupting the step will not result in a broken (partially
patched) tree.

Steps are triggered on the following:
- download: parameters changed
- prepare: download step ran, patch overlay contents changed, patch list and/or contents changed, patch command changed
- configure: prepare step ran, command line changed, koreader/toolchain file contents changed, direct dependency change
  (e.g. a zlib's version change — archive MD5 or Git revision — trigger a re-build of libpng)
- build: configure step ran, command line changed, source file timestamp changed, direct dependency install step ran
- install: build step ran, command line changed
We don't need it (not yet, since we don't use meson).
We need a more recent make anyway (or the newly added version check won't be happy).
- drop unneeded: gettext, gnu-getopt, grep, wget
- add new one for updated build system: findutils & util-linux
Will cleanup output directory (keeping cmake and thirdparty
directories), as well as thirdparty project install stamp files
and trigger a build.
Use `$(MAKE_HOST)` variable instead of uname.
@Frenzie Frenzie merged commit f925273 into koreader:master Jul 15, 2024
3 checks passed
@benoit-pierre benoit-pierre deleted the pr/autonuke branch July 15, 2024 19:52
Makefile.defs Show resolved Hide resolved
@poire-z
Copy link
Contributor

poire-z commented Jul 20, 2024

Just reporting that I updated base & front on my small linux, and managed to rebuild the emulator, after installating ninja and kodev clean (tried first rm $(find . -name CMakeCache.txt) and rm -rf $(find . -type d -name CMakeFiles) as suggested by the build tool, but that was too much or not enough and it didn't build - so: kodev clean).
Took 25mn, no issue (except that all my koreader-emulator-x86_64-linux-gnu-debug/koreader/ content, settings, gestures, patches, styletweaks got removed :/ happened to me once, stupid I am).

(Also stupid I got to do it on this really warm day, I feared for my fanless PC that got hyper warm.)

@benoit-pierre
Copy link
Contributor Author

So how long does a make or make koreader-cre invocation with no source change takes now?

@poire-z
Copy link
Contributor

poire-z commented Jul 20, 2024

Via my wrapper scripts (which do kodev build or make koreader-cre):

$ time ../emu_build_koreader.sh
make[1]: Entering directory '/space/kobo/koreader/base'
⸠ 0% | Building 'crengine'
ninja: no work to do.
⸠50% | Building 'koreader'
ninja: no work to do.
â¸100% | Installing 'koreader'
make[1]: Leaving directory '/space/kobo/koreader/base'
[*] create symlink instead of copying files in development mode
[*] install front spec only for the emulator
[*] Install update once marker
[*] Install plugins
[*] Install resources
real 0m1.369s
$ time ../emu_build_crengine.sh
make[1]: Entering directory '/space/kobo/koreader/base'
⸠ 0% | Building 'crengine'
ninja: no work to do.
⸠66% | Performing build step for 'koreader-cre'
ninja: no work to do.
make[1]: Leaving directory '/space/kobo/koreader/base'
done.
real 0m0.740s

So, well done :)

But now, any crengine single file rebuild outputs a lot of warnings - that I was fine without :). Any way to remove that, so I can only see what's being rebuild without much noise ?

@benoit-pierre
Copy link
Contributor Author

That's because crengine is now compiled with -Wall. This was supposed to be part of another PR but included by mistake in #1849. I have an incoming PR to fix those warnings.

@jospalau
Copy link

I have a problem since this update when compiling koreader-cre:
cp: cannot open '/root/koreader/base/thirdparty/lunasvg/build/downloads/source/.git/objects/pack/.l2s.tmp_idx_o56Iwa0001' for reading: Too many levels of symbolic links

Checking out the previous commit works.

@benoit-pierre
Copy link
Contributor Author

benoit-pierre commented Jul 20, 2024

I have a problem since this update when compiling koreader-cre: cp: cannot open '/root/koreader/base/thirdparty/lunasvg/build/downloads/source/.git/objects/pack/.l2s.tmp_idx_o56Iwa0001' for reading: Too many levels of symbolic links

First question: why root?

And is /root/koreader/base/thirdparty/lunasvg/build/downloads/source/.git/objects/pack/.l2s.tmp_idx_o56Iwa0001 a symbolic link? What's the target?

You can try removing / renaming /root/koreader/base/thirdparty/lunasvg/build/downloads/source so the git original clone gets recreated.

@benoit-pierre
Copy link
Contributor Author

Also, paste more of the log, running with V=1: make lunasvg V=1.

@jospalau
Copy link

Root because it is a Termux Proot distro. I compile for my Kobos in the phone. I had to go through a few hoops to make it work but it works.

The file is a symbolic link to itself. It happens with other targets like libjpeg-turbo, not just lunasvg.

In any case is not important since I will be using the computer again in September

@benoit-pierre
Copy link
Contributor Author

So a git bug? Which version are you using?

The method used to create the checkout is different, so the error might have been previously ignored.

@jospalau
Copy link

git --version git version 2.43.0

@benoit-pierre
Copy link
Contributor Author

Well, I don't know what's happening. If you want to investigate, you can try manually running the commands used by clone_git_repo (in thirdparty/cmake_modules/koenv.sh), see which one result in that invalid symlink.

@jospalau
Copy link

Cheers. I will take a look and report back if I found out something

@jospalau
Copy link

I skipped the errors commenting the return and works again:
(flock -n 9 && cp -a "${repo}" "${tree}") 9>"${repo}.lock" # || return 1

Thanks

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.

None yet

5 participants