search

Willy-JL / F95Checker

GNU General Public License v3.0
123 stars 20 forks source link

precursor for f95cheker Appimage #122

Closed nazdridoy closed 9 months ago

nazdridoy commented 9 months ago

I'm attempting to create an AppImage package for f95checker (cx_Freeze v6.16 will support bdist_appimage). Everything works more or less without any issues, except for the error on main.py line 95:

with (self_path / "log.txt").open("wb", buffering=0) as log:

This throws an error as the AppDir of AppImages is read-only, preventing f95checker from writing its log to {executable.path}/log.txt. I have successfully resolved this issue by modifying it as follows:

from modules import globals
with (globals.data_path / "log.txt").open("wb", buffering=0) as log:

I plan to submit a pull request with AppImage support once cx_Freeze v6.16 is released.

Please note: In my humble opinion, writing the log file in the executable path is not ideal. Instead, it can be placed inside the data_path (making it easier to find and avoiding permission issues if one wants to package it for a distribution like archlinux {AUR}).

r37r05p3C7 commented 9 months ago

fyi, parser saves borked requests in self_path (example). check for references of self_path in your lsp to see more.

making it easier to find and avoiding permission issues if one wants to package it for a distribution like archlinux

true, but there is no need to publish f95checker on aur, imo, because it has an autoupdater which never failed me before, i'm using arch btw =)

speaking of which, how will autoupdater handle appimage packaging?

nazdridoy commented 9 months ago

fyi, parser saves borked requests in self_path (example). check for references of self_path in your lsp to see more.

yeap fond some more. i believer there are other places where these debug binaries can be stored (without cluttering self_path ). I'll look into it .

modules/api.py
291:            async with aiofiles.open(globals.self_path / "login_broken.bin", "wb") as f:
359:    with tempfile.NamedTemporaryFile("wb", prefix=webpage_prefix, suffix=".html", delete=False) as f:
522:            async with aiofiles.open(globals.self_path / "check_broken.bin", "wb") as f:
731:        async with aiofiles.open(globals.self_path / "notifs_broken.bin", "wb") as f:
829:        async with aiofiles.open(globals.self_path / "update_broken.bin", "wb") as f:

how will autoupdater handle appimage packaging?

It's doable, just like other builds. AppImages can be updated by simply replacing the AppImages (R/W) instead of replacing files in the AppDir or 'self_path' (RO). But first we have to fix these issue (writing into the executable path).

        src = asset_path.absolute()
        dst = globals.self_path.absolute()
        if macos_app := (globals.frozen and globals.os is Os.MacOS):
            src = next(asset_path.glob("*.app")).absolute()  # F95Checker-123/F95Checker.app
            dst = globals.self_path.parent.parent.absolute()  # F95Checker.app/Contents/MacOS
        ppid = os.getppid()  # main.py launches a subprocess for the main script, so we need the parent pid
....................
       else:
            for item in dst.iterdir():
                try:
                    if item.is_dir():
                        shutil.rmtree(item, ignore_errors=True)
                    else:
                        item.unlink()
                except Exception:
                    pass
            for item in src.iterdir():
                try:
                    shutil.move(item, dst)
                except Exception:
                    pass
Willy-JL commented 9 months ago

Honestly, I'm tempted to remove the log altogether. The main thing I wanted to solve with this workaround of having a subprocess was to get logs of things happening in C code aswell, which I have no control over, like qt and opengl messages, if any. But then it's just confusing and doesn't catch everything anyway.

could go back to having just one main process, no log file, have 2 executables as is now for debug and not on windows (one disables the cmd window altogether), while Linux and Mac can just get a command line flag, and expect to be run in their own terminal to see the logs. Also windows can have a "press enter to exit" after checker is done, so you can see the logs after it dies maybe.

appimage would be cool, and so would AUR and maybe fedora COPR and less likely Ubuntu PPA. This would need some changes, first of all need a way to make it so AUR and package manager builds don't try to update on their own, but also don't show "beta" text. And appimage would need an update to the update process to detect its an appimage and replace the file instead of the folder.

as for those files to help with fixing issues, I might incorporate them into the gui. Could just be a spoiler showing request headers and body content for the request that failed. Saving to file was always kinda weird, and often didn't help

nazdridoy commented 9 months ago

accidentaly closed the PR. i'm creating a new PR with modification to how f95checker handles log file and largly avoide writing to executable path.

nazdridoy commented 9 months ago

Tempted to remove the log altogether.

If you think it's okay, sure. But I like to have as many logs as I can.

No log file, have 2 executables as is now for debug.

Rather than having 2 executables, if you could make it always write to a logfile (with a file size limit) and overwrite it when f95checker restarts. I myself had to fix an obscure bug in my DE config to run it. Without the log, it would have been a nightmare. Having two executables creates issues with packaging as an AppImage too, as I have to modify cx_freez AppImage build to be able to run both executables with a single AppImage. Otherwise, we have to build two AppImages. (Command line debug flags can also be an awesome alternative for Win/Unix.)

AUR and package manager builds don't try to update on their own.

Doesn't seem like that much of an issue. If you want to make sure to do something right after an upgrade, post-install hooks can probably be useful. Also, the auto-update feature probably has to go. (Which should not be an issue.)

AppImage would need an update to the update process to detect it's an AppImage and replace the file instead of the folder.

I'll submit a PR to add that when cx_freeze v6.16 gets released.

I might incorporate them into the GUI.

Being a Linux guy, I love my log files. Please don't take those away from me. :3

btw, @Willy-JL thank you for making this tool.

Willy-JL commented 9 months ago

Rather than having 2 executables, if you could make it always write to a logfile (with a file size limit) and overwrite it when f95checker restarts. I myself had to fix an obscure bug in my DE config to run it. Without the log, it would have been a nightmare. Having two executables creates issues with packaging as an AppImage too, as I have to modify cx_freez AppImage build to be able to run both executables with a single AppImage. Otherwise, we have to build two AppImages. (Command line debug flags can also be an awesome alternative for Win/Unix.)

The point is that there is no real way to capture all logs correctly. And the 2 executable stuff is only for windows. On windows, if you open a Python program that has console output, it shows a terminal window. That's not ideal. So windows has 2 executables, one for no console, one for console + debug logs in it. Currently, on Linux there are also 2 executables, the only different being that debug is enabled, showing more logs. What I was saying is:

nazdridoy commented 9 months ago
  • no logs to file since they aren't really possible with compiled Python

i was kidding about the log files.

:3

r37r05p3C7 commented 9 months ago

Also, the auto-update feature probably has to go. (Which should not be an issue.)

what about Windows users?

and so would AUR and maybe fedora COPR and less likely Ubuntu PPA. This would need some changes, first of all need a way to make it so AUR and package manager builds don't try to update on their own, but also don't show "beta" text.

Many packages on AUR have both stable and git versions. We should consider following this convention. To avoid adding new build steps and generating additional artifacts, I propose adding a simple flag to disable auto-update, "beta" message can be kept since git package will be separate. This flag can be toggled using any text editor in the PKGBUILD, similar to the debug flag we already have.

As for COPR and PPA, I believe it would be a waste of resources. An AppImage with auto-update enabled would offer almost the same level of convenience without the need to build and maintain deb/rpm packages.

r37r05p3C7 commented 9 months ago

also, the extension will be inaccessible from the appimage, so I suggest excluding it if possible, to reduce file size.

Willy-JL commented 9 months ago

i have some ideas about how to make it all work, dont worry guys :D

nazdridoy commented 9 months ago

what about Windows users?

thay will have autoupdate. it'll be disabled for linux packages

Many packages on AUR have both stable and git versions

for AUR that was my plan, but if we want push release version as well then we have to tweak things a bit.

An AppImage with auto-update enabled would offer almost the same level of convenience without the need to build and maintain deb/rpm packages.

that, i almost agree with

also, the extension will be inaccessible from the appimage, so I suggest excluding it if possible, to reduce file size.

yeah, those has to be excluded. infact if we are going to build appimage or f95checker-version.pkg.tar.zst its kinda unnecessery to include extentions with the program. links for downloading extention can be serve to the user using the gui.

tbh, as @Willy-JL was releasing f95checker like yet another Python project, which is precompiled (loosely speaking) for ease of use, it has way too many things packaged with it. Ideally, a release should only contain a single binary, be it a .exe, .AppImage, .deb, .rpm, or any other package format. All the other pieces (extensions and other addons, if there are any) should be pulled (through the GUI) or pushed (via GitHub pages or releases) as they are needed.

If no one does, i'm very likely going to make a PKGBUILD for AUR.

I'm too spoiled to not run

 paru -S windows11

:P

Willy-JL commented 9 months ago

yeah, those has to be excluded. infact if we are going to build appimage or f95checker-version.pkg.tar.zst its kinda unnecessery to include extentions with the program. links for downloading extention can be serve to the user using the gui.

nope. check out 6c9d92d0c55e32574d14babb270bbddedc37e2ec...3f7fa043ae0d248c38cb95f1bdaec34a43745541

nazdridoy commented 9 months ago

nope. check out 6c9d92d...3f7fa04

looked promising, but I don't think this would work. besides loading a browser extention through a program sounds like a scary thing for windows and all its AV bloat.

ref: https://github.com/Willy-JL/F95Checker/issues/125

nazdridoy commented 9 months ago

also, the extension will be inaccessible from the appimage

no. accessible but ReadOnly at /tmp/.mount_F95Checker{random}/extension