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

Fix broken .mobi build #1656

Merged
merged 1 commit into from
Jun 26, 2021
Merged

Fix broken .mobi build #1656

merged 1 commit into from
Jun 26, 2021

Conversation

HonkingGoose
Copy link
Contributor

@HonkingGoose HonkingGoose commented Jun 15, 2021

Changes

  • Try to fix the broken .mobi build

Context

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

@HonkingGoose
Copy link
Contributor Author

@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
Copy link
Member

jnavila commented Jun 15, 2021

Is this still work in progress?

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

@HonkingGoose
Copy link
Contributor Author

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
Copy link
Member

jnavila commented Jun 15, 2021

@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
Copy link
Contributor

slonopotamus commented Jun 15, 2021

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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...

@HonkingGoose HonkingGoose deleted the patch-2 branch June 23, 2021 10:24
@slonopotamus
Copy link
Contributor

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 HonkingGoose restored the patch-2 branch June 23, 2021 11:01
@HonkingGoose HonkingGoose reopened this Jun 23, 2021
@HonkingGoose
Copy link
Contributor Author

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

@slonopotamus
Copy link
Contributor

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

@HonkingGoose
Copy link
Contributor Author

@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
Copy link
Contributor

$ 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 HonkingGoose marked this pull request as ready for review June 23, 2021 11:23
@HonkingGoose
Copy link
Contributor Author

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
Copy link
Contributor

It's good.

@slonopotamus
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Member

ben commented Jun 26, 2021

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.

@ben ben merged commit 50311d8 into progit:master Jun 26, 2021
@HonkingGoose HonkingGoose deleted the patch-2 branch June 26, 2021 15:47
@HonkingGoose
Copy link
Contributor Author

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! 😄

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.

4 participants