search

MRepoApp / magisk-modules-repo-util

A util for building modules repository
GNU General Public License v3.0
121 stars 25 forks source link

Add tracks.json #3

Closed IzzySoft closed 1 year ago

IzzySoft commented 1 year ago

With a growing repo it might be easier to keep track of modules if one could configure them in separate JSON files, one per repo, and named e.g. by their ID. Those files could just contain the basic info which is then "compiled" into json/modules.json. It would be great to also have some timestamps there, too, for maintenance purposes. Picking an example from the Readme here:

modules/zygisk_lsposed.json:

{
  "id": "zygisk_lsposed",
  "update_to": "https://lsposed.github.io/LSPosed/release/zygisk.json",
  "license": "GPL-3.0-only",
  "added": "2022-10-21",
  "last_update": "2023-01-23",
  "versions": 2
}

The timestamps would allow to spot modules having received no updates "for ages" – and the versions (to be increased whenever a new version was fetched successfully) help identify very actively maintained ones. What "time format" to use (just the date as above, or datetime, or a Unix timestamp) might be worth thinking about, but IMHO date-as-above would suffice.

As indicated, I'd suggest to place those per-module JSON into the modules/ directory. Not sure if you'd see that as a conflict to the name of the json/ directory name, but mixing them in there could become a bit confusing (and make parsing harder as you had to skip those two JSON files when scanning the per-module JSONs).

PS: Please do not delete solved issues, just close them – they can be helpful in the future for references. I just wanted to look up something in our initial issue here (#1, "Refactor magisk-modules-repo-util") and found it was gone :cry: I wanted to look/ask there about the "current final state", especially concerning support for the format of "established repos" – to know if it makes sense to start on my repo already, or if I had to wait for MRepo being ready for the refactored format.

SanmerDev commented 1 year ago

I'm not sure if these fields will be useful, I also found that some fields are redundant when refactoring MRepo. When my repo becomes very complicated, and the established repos are very different from it, then I have to reconsider their compatibility issues, which will affect my refactoring of MRepo. There are indeed many problems😶‍🌫️, you can wait to build your repo. I'm currently working on refactoring MRepo, Sync will not be adjusted until then.

IzzySoft commented 1 year ago

I'm not sure if these fields will be useful,

They were just a suggestion (taken from what I'm used to from F-Droid, plus the counter). If "unknown fields" are simply ignored, that's also OK – I could then place "added" and my personal comments where needed. My main concern was modularity – as with 50+ modules in the repo it becomes harder to handle (my current list already has 14 entries).

then I have to reconsider their compatibility issues

If that's meant to say that the current format is understood by the other clients already (and no separate JSON is needed) I'd agree. But as far as I understood, that's not the case. Apart from that, I didn't mean these additional fields must end up in modules.json: with per-module-json there'd be no need to touch modules.json manually, so edits would happen to the per-module JSON – and what ends up in modules.json is a completely different thing. It could include these additional fields (if unknown fields are ignored by clients) or not.

you can wait to build your repo. I'm currently working on refactoring MRepo, Sync will not be adjusted until then.

Thanks! Waiting for your signal then.

SanmerDev commented 1 year ago

Adding new fields or new JSON will not affect MRepo, I use Retrofit for JSON data requests, it is very powerful and I can not parse fields that are not currently needed. After the MRepo refactoring is completed, this part will be newly processed, and the UI and logic have been modified a lot, which took a lot of time, coming soon🤪

SanmerDev commented 1 year ago

I plan to add a tracks.json in json/, the data structure will take the example you gave.

This should not be user facing, this should be useful to repo owners and developers.

IzzySoft commented 1 year ago

As you give the name tracks.json it looks like a single file. Just to make sure: I meant a single JSON file per module listed in the repo. So if my repo hosts 50 Magisk modules there'd be 50 JSON files, each "tracking" one of the Magisk modules.

Do we both mean the same?

SanmerDev commented 1 year ago

One trace.json per module? , just like update.json. My thought is to have only one tracks.json like modules.json, but one per module seems better.

IzzySoft commented 1 year ago

one per module seems better.

Yupp, easier to maintain I'd say (running my own F-Droid repo with 1k+ apps, and being a maintainer of F-Droid.org with more than 4k apps, where apps are dealt with that way, I guess I can tell :smile:) Glad you like the idea!

May I ask where we stand? I just noticed 2 updates to the app itself, which looks like you made a lot of progress. Looking forward to your "ping" telling me it's time to start with my own modules repo :smiley:

SanmerDev commented 1 year ago

In the past, I have been working on improving MRepo. Now, I will start this part next, and I believe we will see it in the next version😋.

IzzySoft commented 1 year ago

Cool! Eagerly looking forward to that. And yes, first things first – modularization here won't change the "interface" anymore, it's "just" making back-end processing easier :smiley:

PS: My queue of modules is steadily growing. Currently 17 and counting (only those with update.json – before approaching those without I wanted to have everything else up and running). Thanks for sticking to it!

IzzySoft commented 1 year ago

I see you closed this issue as "completed". Some questions left after looking at the Readme:

IzzySoft commented 1 year ago

PS: I very much hope the pygithub dependency is not needed (and there's a way to get the repo-util running without it), as that would be a deal-breaker for me otherwise. Not only is pygithub itself 4 MB, but also has a lot of additional dependencies (so I'm probably ending up with 10 MB dependencies I don't want).

I'm quite "down" at the moment: when I found your project it looked like a nice and simple way to get a Magisk Module repo running, I was so enthusiastic. Now it grew so complex with its dependencies that it makes no longer sense for me. I cannot go with a previous incarnation either, as then there'd be no compatible Android client, which would make my repo useless. :cry:

SanmerDev commented 1 year ago

track.json

{
  "id": "zygisk_lsposed",
  "update_to": "https://lsposed.github.io/LSPosed/release/zygisk.json",
  "license": "GPL-3.0",
  "added": 1679025505.129431,
  "last_update": 1679025505.129431,
  "versions": 1
}
IzzySoft commented 1 year ago

Ah, thanks @ya0211 – so there's still hope! I was quite frustrated yesterday thinking all my hopes need to be abandoned…

I haven't updated the README yet.

That explains why I missed the pointers. Some short hints on first-time setup would be really great.

At present, pygithub is only used when I create a mirror of the repo on GitHub. It is really useless for the sync of the traditional mode (hosts.json).

That confirms my guesses, to my relief :smiling_face_with_tear:

I will find a way to reduce their coupling.

That'd be really great! Maybe only importing it when the config points out it's needed – i.e. repo_branch is set and sync_mode is git) – so of someone doesn't use Github for this would not need to have the pygithub module installed? And can you let me know when that's possible, so I check again?

util does exist git log rewrite and force push.

Yes. Utils do exist to :bomb: and :fire:, too – but that does not mean one should use them lightly (oops). Especially it's strongly recommended to NOT use rewrite for something already published (pushed) and thus might be forked/cloned by others. There are rare cases where it's needed, e.g. if credentials were accidentally published and the corresponding commits must be removed. Such a rewrite breaks all forks and clones, including their branches – they won't be able to pull your changes anymore, but would need to do a "fresh start" and re-apply all the changes they've made, including lots of manual work with diff/patch as a rebase won't be possible.

IzzySoft commented 1 year ago

PS: Shouldn't the timestamp (from update.json) also be part of modules.json as well? Or how does the MRepo client obtain those details to present them to the user (to see when the module itself was last updated/released)? Where does the corresponding date shown in "versions" come from? I don't see that in modules.json. E.g. the 2023-02-22 here:

If it's not in modules.json it probably needs an extra request to retrieve it from the update.json of the corresponding module – which again requires a network connection when browsing a repo. Having all the version details in modules.json would make that unnecessary (as with F-Droid, just the index – here modules.json – would be needed to kept up-to-date on-device, and all details were there). Just wondering.

Further, when setting up a new module, would it now suffice to create the corresponding directory and place the corresponding track.json there, so it would be picked up then? But maybe I should wait for your update to the Readme before asking all the details :wink:

SanmerDev commented 1 year ago
IzzySoft commented 1 year ago

I made some changes and updated REMAD.md

Thanks! Yes, a good help to getting started. A few notes, though:

Next I checked if I can add a new module by simply creating its directory and placing a corresponding track.json inside (this was the idea behind the "one JSON per module") – but that doesn't seem to work: all I get then is a WARNING: clear removed modules: <module_name>. So it seems everything in modules/ shall not be touched manually, but one needs to edit hosts.json? Idea was to easily add a module by creating the directory and dropping in its JSON (and remove it the very same way). Else with a lot of modules, maintenance becomes error-prone (the input of --add-module is hard to automate, dropping in a JSON is easy – and there is not even a --remove-module).

So why not instead of having an additional hosts.json, partly duplicating the content of the track.json but having no additional details) simply use the track.json files for this? Would make maintenance much easier :smiley:

hosts.json was moved to config, which I think serves its purpose

Is hosts.json really needed at all (see above)? Feels somehow redundant. Its contents can easily be produced by parsing the corresponding files from within the modules/ directory.

pygithub is no longer required to be installed, it is optional

Yes, I can confirm it is not missed :partying_face: Thanks a lot!

repo_branch and sync_mode are no longer needed in config.json, and the git push will be done by cli.py -p (the branch used will be read by itself or defined by the user), and other push methods will be done by the user (after cli.py finishes syncing)

Great! Makes the config look much clearer. Only few users will need multiple branches I guess, if at all.

2023-02-22 comes from the timestamp in update.json. I don’t think this part should exist in modules.json. If the user views the module, he may want to install or update the module or view the changelog, the network connection is alway necessary

If that's your intention, I'll accept that. But I do not fully agree. You know I run a big F-Droid repo (with more than 1.000 apps) and am also one of the maintainers at F-Droid.org (where the repo houses more than 4.000 apps). We keep all those data in our JSON index (except for the binaries like graphic files or the APKs themselves). The required files (here: changelogs, Readmes) are fetched from their sources at build time and directly integrated. So users can browse the repo without any network connection required (very helpful in several situations – one example a "boring train journey" with no network available, where one can use the time to browse through available modules and read their descriptions), just graphics would be missing then.

Well, but that can also be something for later (if you want me to, I can move that topic to a separate issue to ponder on).

Currently track.json and update.json are located in the same directory, more information can be found in README.md

That's totally fine! Everything belonging to a specific module can be found in one place. So if I want to remove a module for some reason, I could simply remove that directory (in my original idea; not working currently as hosts.json is used for control – which is why I asked above to "obsolete" that; again something that can be moved to its own issue if you want).

Summing up: a big thanks for all your work, @ya0211 – looks like I'm soon ready to start with my repo. Just need to know which files to sync. Am I assuming correctly that just the json/ and modules/ directories would suffice? MRepo most likely just fetches the modules.json and what's linked from there, right?

SanmerDev commented 1 year ago

Summing: @IzzySoft Thank you very much for your attention to this project and the suggestions you provided. This is an opportunity for me to learn Android development, and it has developed far beyond my imagination. Although I don't major in software engineering, but because of the experience of this project, I think I will soon enter this industry 🥳.

IzzySoft commented 1 year ago

I'd add a check for the repo_url

Cool, thanks! Not a prio-1, but good to have (can avoid some confusion).

cli.py --add-module： Indeed, it's ambiguous, I'll fix it

:+1: Great!

cli.py -u <username> --no-sync: IIn fact, my intention is that as one of the three ways to generate hosts.json, I will improve the README.md

:smiley:

crashes as it cannot find json/modules.json: I think it came from Repo.py#L38 and I'll fix it

Yepp, that sounds like it. If no module was added yet, there's nothing to load. So maybe simply create a new one if the file is not (yet) there.

WARNING: clear removed modules: <module_name>: When you use debug mode(cli.py --debug), it will no longer be a warning, it will directly throw the error

Well, one usually only runs in debug mode when debugging. I was rather thinking along the lines of "throw a warning but do not automatically remove". Like WARNING: detected possibly removed module '<module_name>'. Run 'cli.py --remove-unused' to remove it. (and have that command available). Else data might be lost due to some typo or whatever.

cli.py --remove-modul: it will come with the next commit

Great! That piece was really missing :smile: Just take care for consistent spelling: it should be --remove-module (with a trailing e) :nerd_face:

hosts.json & track.json: I think they can be merged into tracks.json, we no longer need hosts.json and track.json of each module, and add --print-track to cli.py

Wait, that might go the wrong way. Do you still have in mind one could manipulate existing modules by simply adding their directory or removing it? track.json contains the details needed for that. So this file should exist per-module. So to add the module izzy_example, I'd just do the following:

mkdir -p modules/izzy_example
cat > modules/izzy_example/track.json <<EOF
{
  "id": "izzy_example",
  "update_to": "https://izzy.example.com/update.json",
  "license": "WTFPL"
}
EOF
util/cli.py

(though I'd rather replace the cat by something like jo -p id=izzy_example update_to=https://izzy.example.com/update.json license=WTFPL to make sure it's proper JSON) and the module would be added (who prefers --add-module can still use that). And to remove it, all I had to do would be

rm -rf modules/izzy_example
util/cli.py

(though for that, --remove-module would be fine, too). Plus I could easily adjust a single module without much risk to modify the wrong one e.g.

sed -i 's/"license": "WTFPL"/"license": "Unlicense"/' modules/izzy_example

That would make automated maintenance much easier! And also avoid adding "duplicate IDs" modifying modules.json by hand as the inter-active --add-module cannot be scripted easily :see_no_evil:

modules.json & update.json: Regarding this issue, I think I have to consider that it will be related to the change of the client. But it is undeniable that the current method is a waste of the user's network, also increases the loading time of the page.

Yes, exactly. But that's something that can be postponed if you prefer (though then for a while the client would need to support both formats; so the earlier this is decided, the less the impact).

Summing:

:rofl: Yeah. Glad to be of assistance – and even more glad to accept the challenge! I wouldn't have the time to implement all that – but I can (and do gladly) share from my experience running/maintaining larger repos.

SanmerDev commented 1 year ago

New

IzzySoft commented 1 year ago 
$ util/cli.py 
  File "util/cli.py", line 238
    match option:
          ^
SyntaxError: invalid syntax

Python 3.8.10 here. match was only added with 3.10. And no, I currently cannot upgrade, as that would mean a ton of dependencies on the given machine (entire dist upgrade; there's a reason I'm using LTS :wink:) Maybe one of the alternatives can be used instead? As it's just 3 cases, I guess the "if .. elif` should be easiest

    while True:
        if option == 1:
                print_header("Add Module")
                option = add_module()
        elif option == 2:
                print_header("Remove Module")
                option = remove_module()
        elif option == 0:
                print_header("Modules Manager")
                print_value(1, "Add Module")
                print_value(2, "Remove Module")
                option = input_optional("Option", "[1/2/q]: ", [1, 2])
        print()

Works so far then. But now I get some other errors on sync:

Sync ERROR: zygisk_lsposed: update module failed: TypeError(unsupported operand type(s) for *: 'NoneType' and 'float')

Well, "for *" is quite precise. Which asterisk should it be? That's for modules added with the previous version btw. I just added a new one, and the error looks different:

update module failed: JSONDecodeError(Expecting value: line 8 column 1 (char 7))

Tracking it down with debug (-d), the pointer is

File "util/sync/Repo.py", line 246, in pull

Adding some more debug output I can pin it to versions_item = self._update_module(item, host) failing. The passed arguments seem fine. Digging deeper with some more added debug print() lines, I was able to nail that one – again my "good old" Gihub-link error (/blob/ URL instead of /raw/). To make that easier to catch, it would help to have another check in _get_module_from_json, or rather an exception handler. The failing command is:

update_json: AttrDict = load_json_url(host.update_to)

not resulting in JSON with /blob/ URLs. So it should be

        update_json: AttrDict = load_json_url(host.update_to)
        try:
            downloader(update_json.zipUrl, tmp_file)
        except ...
            // log message: URL did not return valid JSON

or maybe catch that in load_json_url?

Btw: on each call I see

fatal: not a git repository (or any parent up to mount point /)

Do you try syncing with git, despite no git repo is set up? Guess you didn't notice that as you're using git there :see_no_evil:

OK, I'll now check why the other two modules I added before were not working after the update to your newest commit.

IzzySoft commented 1 year ago

Hm, here's the diff (diff -u new_track.json old_track.json):

   "changelog": "",
-  "last_update": 1679527941.189307,
+  "added": 1679357931.571593,
+  "last_update": 1679357931.571593,
   "versions": 1

so the old one had an additional field which you now dropped. But shouldn't that be ignored when interpreting the JSON object? Just checked: Removing that line, the errors are gone. Not sure how many people use the code already so it would be worth the effort to check and clean that as well?

But COOOL! This now works:

mkdir modules/systemless_debloater
jo -p -- id="systemless_debloater" update_to="https://github.com/Magisk-Modules-Alt-Repo/SystemlessDebloater/raw/main/update.json" license="LGPL-2.1-only" -s changelog="" > modules/systemless_debloater/track.json
util/cli.py sync

:partying_face:

As well as

rm -rf modules/systemless_debloater
util/cli.py sync

Wonderful! :star_struck:

SanmerDev commented 1 year ago 
mkdir modules/systemless_debloater
jo -p -- id="systemless_debloater" update_to="https://github.com/Magisk-Modules-Alt-Repo/SystemlessDebloater/raw/main/update.json" license="LGPL-2.1-only" -s changelog="" > modules/systemless_debloater/track.json
util/cli.py sync

can be replaced by 

util/cli.py config -a id="systemless_debloater" update_to="https://github.com/Magisk-Modules-Alt-Repo/SystemlessDebloater/raw/main/update.json" license="LGPL-2.1-only" changelog=""

also have 🥳

 util/cli.py config -r systemless_debloater
IzzySoft commented 1 year ago

Remains the update module failed: JSONDecodeError(Expecting value: line 8 column 1 (char 7)) when the server does not return valid JSON, where an exception handler is needed (I meanwhile recognize the error as what it is and immediately know what to do – new users probably won't).

SanmerDev commented 1 year ago
IzzySoft commented 1 year ago

I still don't understand this error about track.json

I can no longer reproduce as I didn't keep the old copies. So maybe let's drop this one (until someone else shows up with the same).

IzzySoft commented 1 year ago

OK, here come some nice pics:

SanmerDev commented 1 year ago

I noticed that there seems to be a gap between the bottom nav bar and the floating button 🤨

IzzySoft commented 1 year ago

Indeed, now that you say it: I just checked whether it's just the screenshot, but no, it shows on-device as well. But still: nice to see my repo in there with 17 modules. Wll probably make it public soon™ – once I found some time for a little care on the web server end (at least having a basic browser front-end would be nice).

SanmerDev commented 1 year ago

What device and Android version is this? There seems to be a issue with the UI when useing the virtual navigation bar.🤔

IzzySoft commented 1 year ago

Wileyfox Swift running Android 10 (LineageOS).

SanmerDev commented 1 year ago

Ok, I'll check this out.