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

Hack for mirrobrain magnet URIs #1001

Merged
merged 2 commits into from
Oct 9, 2023
Merged

Hack for mirrobrain magnet URIs #1001

merged 2 commits into from
Oct 9, 2023

Conversation

rgaudin
Copy link
Member

@rgaudin rgaudin commented Oct 2, 2023

As per #938, in regard to kiwix/container-images#242, this adds some processing to the creation of the magnet: link for downloads.

It mostly fixes the parameter names/values of the magnet string but also fetches the list of mirrored URLs (using metalink) to add them as alternative webseeds.

Tested with Vuze/Azureus on Windows as well as Tixati on Windows (Tixati is smart enough to retrieve metalink from the webseed and thus didn't even need the extra ws=).

Please note that while Transmission understands it's multiple webseeds, there's no support for fetching the torrent/DHT and so the download never starts. Don't be fooled 😉! I guess it's similar in most other torrent client (most of them dont support webseed magnets).

Suggestion: target="_blank" for magnet is really annoying (leaves a blank tab open). I think it should be removed.

static/skin/index.js Outdated Show resolved Hide resolved
static/skin/index.js Outdated Show resolved Hide resolved
@rgaudin rgaudin requested a review from veloman-yunkan October 4, 2023 16:17
static/skin/index.js Outdated Show resolved Hide resolved
static/skin/index.js Outdated Show resolved Hide resolved
@rgaudin rgaudin requested a review from veloman-yunkan October 6, 2023 09:07
@rgaudin
Copy link
Member Author

rgaudin commented Oct 6, 2023

@veloman-yunkan thank you ; is any of the commit history worth keeping? I think it can all be squashed into one. WDYT?

@veloman-yunkan
Copy link
Collaborator

@veloman-yunkan thank you ; is any of the commit history worth keeping? I think it can all be squashed into one. WDYT?

@rgaudin I concur with you on this. In fact I wanted to suggest that in my approval but didn't want to appear too finicky. :)

@rgaudin rgaudin force-pushed the magnet branch 2 times, most recently from e5fad92 to b41d436 Compare October 6, 2023 11:58
@rgaudin
Copy link
Member Author

rgaudin commented Oct 6, 2023

Done ; it's rebased off main as well. Thank you for your time.

Missing parameters are added to the magnet link for it to work properly
given it is mainly based on webseeds.
Each mirror serving the file is added as a webseed.
Params that requires URIEncoding are now encoded.

Introduces `makeURLSearchString()` to turn SearchParams into a string but only
URIEncoding some specific params.
@kelson42
Copy link
Collaborator

kelson42 commented Oct 8, 2023

@veloman-yunkan @rgaudin @mgautierfr Unfortunately I can not merge because the CI is red and the error seems worrying.

@mgautierfr
Copy link
Member

The cacheid has to be adapted as static files have been changed.
See Static files compilation section in the README.

@codecov
Copy link

codecov bot commented Oct 9, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 38.92%. Comparing base (2650cdd) to head (ab0d7b6).
Report is 234 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1001   +/-   ##
=======================================
  Coverage   38.92%   38.92%           
=======================================
  Files          58       58           
  Lines        3990     3990           
  Branches     2201     2201           
=======================================
  Hits         1553     1553           
  Misses       1090     1090           
  Partials     1347     1347           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rgaudin
Copy link
Member Author

rgaudin commented Oct 9, 2023

@mgautierfr thanks for the explanation. @kelson42 it's now green

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