"cli.py sync" crashes on comparing 'str' and 'int' for versionCode

[IzzySoft]

I just tried with the latest code from git, and have another crash with cli.py sync:

[2023/08/30 20:46:24] Pull DEBUG: from_track: [NoProcStatRestriction] -> type: ONLINE_JSON
Traceback (most recent call last):
  File "/sanitized/magisk/util/cli.py", line 26, in <module>
    sys.exit(Main.exec())
  File "/sanitized/magisk/util/sync/cli/Main.py", line 53, in exec
    code = cls._check_args()
  File "/sanitized/magisk/util/sync/cli/Main.py", line 73, in _check_args
    return cls.sync()
  File "/sanitized/magisk/util/sync/cli/Main.py", line 265, in sync
    sync.update(
  File "/sanitized/magisk/util/sync/core/Sync.py", line 134, in update
    online_module = self._update_jsons(track=track, force=force)
  File "/sanitized/magisk/util/sync/core/Sync.py", line 38, in _update_jsons
    online_module, timestamp = self._pull.from_track(track)
  File "/sanitized/magisk/util/sync/core/Pull.py", line 242, in from_track
    return self.from_json(track, local=False)
  File "/sanitized/magisk/util/sync/core/Pull.py", line 174, in from_json
    if not self._check_version_code(track.id, update_json.versionCode):
  File "/sanitized/magisk/util/sync/core/Pull.py", line 55, in _check_version_code
    if len(update_json.versions) != 0 and version_code > update_json.versions[-1].versionCode:
TypeError: '>' not supported between instances of 'str' and 'int'

This seems to be a regression from the last commit(s) as it didn't happen with the code from the tag. The culprit probably lies in upstream's update.json I guess:

{
    "version": "v0.0.2",
    "versionCode": "2",
    "zipUrl": "https://github.com/Magisk-Modules-Alt-Repo/NoProcStatRestriction/releases/download/v0.0.2/NoProcStatRestriction_v0.0.2.zip",
    "changelog": "https://github.com/Magisk-Modules-Alt-Repo/NoProcStatRestriction/releases/tag/v0.0.2"
}

As you can see, it defines versionCode as a string. Changing the indicated line 55 in Pull.py to

        if len(update_json.versions) != 0 and int(version_code) > update_json.versions[-1].versionCode:

solves this. But then there's the next crash with another module, unrelated to the above:

[2023/08/30 20:58:26] Pull DEBUG: from_track: [zygisk_lsposed] -> type: ONLINE_JSON
Traceback (most recent call last):
  File "/sanitized/magisk/util/cli.py", line 26, in <module>
    sys.exit(Main.exec())
  File "/sanitized/magisk/util/sync/cli/Main.py", line 53, in exec
    code = cls._check_args()
  File "/sanitized/magisk/util/sync/cli/Main.py", line 73, in _check_args
    return cls.sync()
  File "/sanitized/magisk/util/sync/cli/Main.py", line 265, in sync
    sync.update(
  File "/sanitized/magisk/util/sync/core/Sync.py", line 134, in update
    online_module = self._update_jsons(track=track, force=force)
  File "/sanitized/magisk/util/sync/core/Sync.py", line 38, in _update_jsons
    online_module, timestamp = self._pull.from_track(track)
  File "/sanitized/magisk/util/sync/core/Pull.py", line 242, in from_track
    return self.from_json(track, local=False)
  File "/sanitized/magisk/util/sync/core/Pull.py", line 188, in from_json
    online_module = self._from_zip_common(track.id, zip_file, changelog, delete_tmp=True)
  File "/sanitized/magisk/util/sync/core/Pull.py", line 148, in _from_zip_common            
    changelog_url = self._get_file_url(module_id, target_changelog_file)
  File "/sanitized/magisk/util/sync/core/Pull.py", line 65, in _get_file_url            
    if not (file.is_relative_to(module_folder) and file.exists()):
AttributeError: 'PosixPath' object has no attribute 'is_relative_to'

Here it seems you've changed the naming of the files, but not in all places. The new ZIP is v1.9.1_6990.zip, but going by its module.prop should rather be v1.9.1_(6990)_6990.zip (corresponding to the previous v1.9.0_(6986)_6986.zip). I've tried renaming the files accordingly and running sync again (so it would store the files once more using the wrong name but find the correct ones), but that didn't work out. As the error is

AttributeError: 'PosixPath' object has no attribute 'is_relative_to'

I've added some debug (print(file)) to see which one it is: magisk/modules/zygisk_lsposed/v1.9.1_6990.md. That file does exist. But indeed PosixPath has no is_relative_to – only PurePosixPath has that. So I try file = PurePosixPath(file), which only changes the error message to AttributeError: 'PurePosixPath' object has no attribute 'is_relative_to'. I'm clueless with this one, and why it only happens with this module – hope you have an idea there.

[SanmerDev]

AttributeError: 'PosixPath' object has no attribute 'is_relative_to'

• pathlib.PurePath.is_relative_to was added in version 3.9.

[IzzySoft]

Well, then we have an issue there: many LTS systems (like mine) are having 3.8 still and no easy upgrade path. Can't you use a different implementation? TBH, I don't even see the reason for that check there:

1. you establish the path for the downloaded file based on module_path, i.e. inside it:
   module_folder.joinpath(online_module.changelog_filename)
2. you send it to that method, which then checks whether the new path is inside module_path:
   if not (file.is_relative_to(module_folder))
   – where else should it be if you explicitly created it inside?

So why not reducing if not (file.is_relative_to(module_folder) and file.exists()): to just if not file.exists()? Or, if you insist, how about this:

if not (file.parents[1] == module_folder and file.exists()):

I've just verified: PosixPath.parents is available in Python 3.8. And that would do the job fine, as the downloaded file must reside in <module_path>/<module_id>/. It's even more precise, as any other location (e.g. directly in module_path, or more than one level deeper) would be wrong, without file.is_relative_to() detecting that.

Any chance for an adjustment here, @ya0211?

[SanmerDev]

@IzzySoft , this has been resolved, no longer using is_relative_to

[IzzySoft]

Thanks! I'll update ASAP (just performed a git pull so I have the current code here already, will test later). Hopefully that was the last "culprit".

Just for info: I (hopefully) finished updating my framework last night. Still needs some testing, but I'm confident to be "back to normal" on the weekend. The only open question remaining is what happened to cli.py sync --remove-unused, and what replaced it. Can you tell me, please?

[SanmerDev]

cli.py sync --remove-unused has been removed because it is useless and unnecessary.

[IzzySoft]

Thanks for confirming! I'll then remove it from my framework as well (admitting I cannot remember ever having used it).