progit / progit2

Pro Git 2nd Edition
Other
5.79k stars 1.9k forks source link

Fix broken .mobi build #1656

Closed HonkingGoose closed 3 years ago

HonkingGoose commented 3 years ago

Changes

Context

Should (at least partially) fix my broken PR I made at #1655...

HonkingGoose commented 3 years ago

@jnavila @slonopotamus can you review this PR? I think this should be better, but I'm not sure if it will 100% fix the problem...

Sorry for creating the mess in the first place! 😞

jnavila commented 3 years ago

Is this still work in progress?

I tried to run it locally but it seems to choke on downloading kindlegen.

HonkingGoose commented 3 years ago

Hmm, maybe the code needs to be adjusted some more?

Quote from https://github.com/asciidoctor/asciidoctor-epub3#usage

You may also produce KF8/MOBI file by setting ebook-format attribute to kf8.

$ asciidoctor-epub3 -D output -a ebook-format=kf8 path/to/book.adoc

When the script completes, the file book.mobi will appear in output directory.

jnavila commented 3 years ago

@slonopotamus just to be sure: is kf8 still based on kindlegen? Downloading from webarchive.org is not a long-term solution, specially in CI systems that do it for each build.

slonopotamus commented 3 years ago

Yes, asciidoctor-epub3 still uses kindlegen because alternative solution to produce kf8 isn't ready yet. When it becomes ready, we surely will drop kindlegen and forget about its existence.

HonkingGoose commented 3 years ago

Hi @jnavila and @slonopotamus, I'm not making any progress... Do you have ideas/help on fixing the problem? Or do you know of somebody who can fix the problem?

HonkingGoose commented 3 years ago

I don't know how to fix this, and in the meantime the build is broken on master.

I'd rather we revert my bad change and let somebody more compentent than me update the rake build script so that the .mobi files work again.

See #1662 for the PR that reverts the original bad change by me...

slonopotamus commented 3 years ago

How do you see that build is broken on master? I see that builds are not running on master: https://travis-ci.org/github/progit/progit2

Since June 15th, 2021, the building on travis-ci.org is ceased. Please use travis-ci.com from now on.

HonkingGoose commented 3 years ago

@slonopotamus Good point! I think we just don't have any functional builds anymore now that travis has shut down the old domain?

slonopotamus commented 3 years ago

Exactly. But you have an issue about that since November 2020 :) #1566

HonkingGoose commented 3 years ago

@slonopotamus Can you try out this PR locally and see if that works/breaks? I don't have a full Ruby setup, so this was basically copy/paste of the stuff we had before... 😄

slonopotamus commented 3 years ago
$ bundle exec rake
Hash on header of contributors list (2ddf197) matches the current HEAD (2ddf197)
Converting to HTML...
 -- HTML output at progit.html
Hash on header of contributors list (2ddf197) matches the current HEAD (2ddf197)
Converting to EPub...
 -- Epub output at progit.epub
Hash on header of contributors list (2ddf197) matches the current HEAD (2ddf197)
Converting to Mobi (kf8)...
 -- Mobi output at progit.mobi
Hash on header of contributors list (2ddf197) matches the current HEAD (2ddf197)
Converting to PDF... (this one takes a while)
 -- PDF output at progit.pdf
Checking generated books
- progit.html
  *  External link https://mvnrepository.com/artifact/org.eclipse.jgit/org.eclipse.jgit failed: 403 No error

HTML-Proofer found 1 failure!
Running ["ImageCheck", "HtmlCheck", "LinkCheck", "ScriptCheck"] on progit.html on *.html... 

Checking 96 external links...
Ran on 1 file!

'htmlproofer --check-html progit.html' failed
Error when checking books (ignored)
$ bundle exec rake book:clean
Removing generated files
rm book/contributors.txt
rm progit.html
rm progit.epub
rm progit.mobi
rm progit.pdf

It produces, it cleans. That 403 error is obviously unrelated. Mobi file itself looks adequate.

HonkingGoose commented 3 years ago

It produces, it cleans. That 403 error is obviously unrelated. Mobi file itself looks adequate.

Thank you very much @slonopotamus!

What do you think? Is this PR good to merge, or do we need to do more work?

slonopotamus commented 3 years ago

It's good.

slonopotamus commented 3 years ago

The process could be improved by only running check_contrib() once (by declaring it a dependency of html/pdf/epub/mobi). But it is out of scope of this PR.

HonkingGoose commented 3 years ago

The process could be improved by only running check_contrib() once (by declaring it a dependency of html/pdf/epub/mobi). But it is out of scope of this PR.

I made a new issue to capture that improvement idea, so we can focus on getting the build functional again first. 😉

HonkingGoose commented 3 years ago

Hi @ben, I think this PR is ready for review/merge.

There are no automated tests on this PR because Travis CI has moved to the new domain (.org -> .com). We also don't create and publish any new files to GitHub Releases because of this.

To get our build working again, we have 2 options:

  1. Migrate from Travis to GitHub Action
  2. Migrate to the .com version of Travis

I think @jnavila was working on a GitHub Action workflow file?

The short-term easy thing to do would probably be to migrate to the .com version, even if it gives us a limited amount of builds.

ben commented 3 years ago

Sounds good. I'm on a road trip, but I might be able to figure out the travis-ci.com transition in the next few days.

HonkingGoose commented 3 years ago

Sounds good. I'm on a road trip, but I might be able to figure out the travis-ci.com transition in the next few days.

I hope you have fun on your road trip! 😄