Skip to content

book2: match ebook release assets by name, not content-type #1615

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

Merged
merged 1 commit into from
Jun 29, 2021

Conversation

peff
Copy link
Member

@peff peff commented Jun 29, 2021

Since progit2 2.1.309 switched to using a GitHub Action to generate releases, it no longer sets the content-type field of the release assets. And thus we fail to find any ebooks to link, since we are looking for assets with "application/pdf", and so on.

Instead, let's do a suffix match against the name of each asset, looking for the appropriate file extension. This works fine, since the names are all obvious ("progit.epub", and so on). And it should be backwards-compatible with older translations that haven't yet picked up the new Actions-based workflow, since they used the same sensible names.

We may revert this later if the assets start generating with correct content-types again. But it looks to be non-trivial (there's some discussion in #1498), so this seems like a good solution in the meantime.

/cc @HonkingGoose

Since progit2 2.1.309 switched to using a GitHub Action to generate
releases, it no longer sets the content-type field of the release
assets. And thus we fail to find any ebooks to link, since we are
looking for assets with "application/pdf", and so on.

Instead, let's do a suffix match against the name of each asset, looking
for the appropriate file extension. This works fine, since the names are
all obvious ("progit.epub", and so on). And it should be
backwards-compatible with older translations that haven't yet picked up
the new Actions-based workflow, since they used the same sensible names.

We may revert this later if the assets start generating with correct
content-types again. But it looks to be non-trivial (there's some
discussion in #1498), so this seems like a good solution in the
meantime.
@HonkingGoose
Copy link
Contributor

Instead, let's do a suffix match against the name of each asset, looking for the appropriate file extension. This works fine, since the names are all obvious ("progit.epub", and so on). And it should be backwards-compatible with older translations that haven't yet picked up the new Actions-based workflow, since they used the same sensible names.

I agree, this seems like a good fix for now. Will probably help the translations as well, as they will probably follow the main English repo with their CI setup, as they can basically copy/paste from the English repo and be done with their CI/releases.

That content-type thing needs to be fixed with the upstream action that ProGit2 uses, or by ProGit2 using another action.

I'm open for suggestion on a better action! 😉

@peff
Copy link
Member Author

peff commented Jun 29, 2021

I don't think the Travis CI is ever going to report back; it's still hooked to travis-ci.org, which shut down about 2 weeks ago. We probably should switch that out for an Action workflow.

I'm going to merge this through in the meantime, since the ebook links are currently broken on the site.

@peff peff merged commit a25657d into main Jun 29, 2021
@peff peff deleted the book-release-asset-names branch June 29, 2021 19:32
@HonkingGoose
Copy link
Contributor

I don't think the Travis CI is ever going to report back; it's still hooked to travis-ci.org, which shut down about 2 weeks ago. We probably should switch that out for an Action workflow.

It's not going to report back, it's on permanent holiday. The poor CI bot is off on a vacation after being hammered with requests all the time. 😄

The ProGit2 Travis builds on travis-ci.org were also not working at all, hence the migration to GitHub Actions. 😉

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.

2 participants