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

When I confirm the update installation after restarting the application, I get a console window with the error The batch file cannot be found. #5

Closed
its-monotype opened this issue Sep 24, 2022 · 21 comments
Assignees
Labels
bug Something isn't working

Comments

@its-monotype
Copy link

sdasdas

@dennisvang
Copy link
Owner

@its-monotype Could you provide some more information regarding your system, and could you describe steps to reproduce the issue?

@its-monotype
Copy link
Author

its-monotype commented Sep 26, 2022

@dennisvang I have Windows 11 21H2 (OS Build 22000.1042), VSCode, PowerShell

  1. python -m venv venv
  2. venv/scripts/activate
  3. moved folder myapp from src to the root because I get an error that Module myapp cannot be found in src and main.py to the root.
    I followed your instructions:
  4. python repo_init.py
  5. ./create_pyinstaller_bundle.bat
  6. python repo_add_bundle.py
  7. incremented APP_VERSION in myapp/settings.py
  8. ./create_pyinstaller_bundle.bat again
  9. python repo_add_bundle.py again
  10. python -m http.server -d temp/repository
  11. mkdir temp/my_app && tar -xf ./temp/repository/targets/my_app-1.0.tar.gz -C ./temp/my_app
  12. Run the app executable from temp/my_app folder

@dennisvang
Copy link
Owner

Instead of moving the myapp folder, I would add src to your python path. VSCode probably has a shortcut for that. Otherwise, see e.g. PYTHONPATH

Furthermore, as mentioned in the readme, the example app must be extracted to (and run from) the directory specified in INSTALL_DIR (see src/myapp/settings.py). By default, INSTALL_DIR points to AppData\Local\Programs\my_app.

If you want to use a different directory, you should modify INSTALL_DIR to point to your temp/my_app. Note, this must be done before creating the first pyinstaller bundle.

@its-monotype
Copy link
Author

Okay, thanks, I'll try that

@its-monotype
Copy link
Author

I re-cloned the repository, everything from scratch, added src to PYTHONPATH, did not change anything, and acted exactly according to the instructions, but unfortunately the same thing here.
Screenshot 2022-09-26 182522
Screenshot 2022-09-26 182637

@its-monotype
Copy link
Author

However if I close this console window and then open app again I can see that app was updated
Screenshot 2022-09-26 183028

@dennisvang
Copy link
Owner

dennisvang commented Sep 27, 2022

@its-monotype After the update attempt, there should be a file called install.log in the INSTALL_DIR.
Perhaps you can find some useful information there.

I guess this error is probably caused by the installation batch file trying to delete itself at the very end, after the installation is done. Also see this discussion. The batch file is a temporary file (with delete=False), so maybe it is cleaned up too soon, for some other reason.

We haven't encountered this issue on any of our test systems, but they all run windows 10.

@dennisvang dennisvang self-assigned this Sep 27, 2022
@dennisvang dennisvang added the bug Something isn't working label Sep 27, 2022
@its-monotype
Copy link
Author

its-monotype commented Sep 27, 2022

Strangely, when I started using pyinstaller with the --onefile attribute, this problem disappeared.

I also want to know if it is possible to somehow make the program restart automatically after a successful update, so you don't have to do it manually?

@dennisvang
Copy link
Owner

dennisvang commented Sep 27, 2022

Strangely, when I started using pyinstaller with the --onefile attribute, this problem disappeared.

@its-monotype That's interesting. I'll have a look at that.

I also want to know if it is possible to somehow make the program restart automatically after a successful update, so you don't have to do it manually?

Restarting automatically should be possible, but it is not supported out-of-the-box. This, too, is a matter of limiting the maintenance burden.

Although I haven't tried this, you could probably do it by adding a windows start call to the end of the installation batch file (defined in the WIN_MOVE_FILES_BAT variable).

For cases like this, tufup provides the option to specify your own install_update function. You could use the default install_update function as an example. This custom function can then be passed into Client.download_and_apply_update() via the install argument.

For example:

def install_update_restart(src_dir, dst_dir, purge_dst_dir=False, exclude_from_purge=None, **kwargs):
    # Custom install function with restart functionality
    ...

and then

...
client.download_and_apply_update(..., install=install_update_restart, ...)
...

A quicker alternative would be to monkey patch the WIN_MOVE_FILES_BAT variable and append the start command that way. See e.g. unittest.mock.patch.

@its-monotype
Copy link
Author

its-monotype commented Sep 27, 2022

@dennisvang Сan you please check out my implementation of the automatic restart after an update? It works but I want to know your opinion.

+ # tufup-example\src\myapp\__init__.py
import logging
+ import pathlib
import shutil
+ import subprocess
+ import sys
+ from tempfile import NamedTemporaryFile
import time
+ from typing import List, Optional, Union

from tufup.client import Client
+ from tufup.utils.platform_specific import (
+     ON_WINDOWS,
+     ON_MAC,
+     WIN_ROBOCOPY_OVERWRITE,
+     WIN_LOG_LINES,
+     WIN_ROBOCOPY_PURGE,
+     WIN_ROBOCOPY_EXCLUDE_FROM_PURGE,
+     run_bat_as_admin,
+     _install_update_mac,
+ )

from myapp import settings

logger = logging.getLogger(__name__)

__version__ = settings.APP_VERSION


def progress_hook(bytes_downloaded: int, bytes_expected: int):
    progress_percent = bytes_downloaded / bytes_expected * 100
    print(f"\r{progress_percent:.1f}%", end="")
    time.sleep(0.5)  # quick and dirty: simulate slow or large download
    if progress_percent >= 100:
        print("")


+ # https://stackoverflow.com/a/20333575
+ WIN_MOVE_FILES_BAT = """@echo off
+ {log_lines}
+ echo Moving app files...
+ robocopy "{src}" "{dst}" {options}
+ echo Done.
+ echo Starting app...
+ start {exe_path}
+ rem Delete self
+ (goto) 2>nul & del "%~f0"
+ """


+ def _install_update_win(
+      src_dir: Union[pathlib.Path, str],
+      dst_dir: Union[pathlib.Path, str],
+      purge_dst_dir: bool,
+      exclude_from_purge: List[Union[pathlib.Path, str]],
+      as_admin: bool = False,
+      log_file_name: Optional[str] = None,
+      robocopy_options_override: Optional[List[str]] = None,
+   ):
+     """
+     Create a batch script that moves files from src to dst, then run the
+     script in a new console, and exit the current process.
+     The script is created in a default temporary directory, and deletes
+     itself when done.
+     The `as_admin` options allows installation as admin (opens UAC dialog).
+     The `debug` option will log the output of the install script to a file in
+     the dst_dir.
+     Options for [robocopy][1] can be overridden completely by passing a list
+     of option strings to `robocopy_options_override`. This will cause the
+     purge arguments to be ignored as well.
+     [1]: https://docs.microsoft.com/en-us/windows-server/administration/windows-commands/robocopy
+     """
+     if robocopy_options_override is None:
+         options = list(WIN_ROBOCOPY_OVERWRITE)
+         if purge_dst_dir:
+             options.append(WIN_ROBOCOPY_PURGE)
+             if exclude_from_purge:
+                 options.append(WIN_ROBOCOPY_EXCLUDE_FROM_PURGE)
+                 options.extend(exclude_from_purge)
+     else:
+         # empty list [] simply clears all options
+         options = robocopy_options_override
+     options_str = " ".join(options)
+     log_lines = ""
+     if log_file_name:
+         log_file_path = pathlib.Path(dst_dir) / log_file_name
+         log_lines = WIN_LOG_LINES.format(log_file_path=log_file_path)
+         logger.info(f"logging install script output to {log_file_path}")
+     script_content = WIN_MOVE_FILES_BAT.format(
+         src=src_dir,
+         dst=dst_dir,
+         options=options_str,
+         log_lines=log_lines,
+         exe_path=dst_dir.joinpath(
+             settings.EXE_PATH.name
+         ),  # settings.EXE_PATH=pathlib.Path(sys.executable).resolve()
+     )
+     logger.debug(f"writing windows batch script:\n{script_content}")
+     with NamedTemporaryFile(
+         mode="w", prefix="tufup", suffix=".bat", delete=False
+     ) as temp_file:
+         temp_file.write(script_content)
+     logger.debug(f"temporary batch script created: {temp_file.name}")
+     script_path = pathlib.Path(temp_file.name).resolve()
+     logger.debug(f"starting script in new console: {script_path}")
+     # start the script in a separate process, non-blocking
+     if as_admin:
+         run_bat_as_admin(file_path=script_path)
+     else:
+         # we use Popen() instead of run(), because the latter blocks execution
+         subprocess.Popen([script_path], creationflags=subprocess.CREATE_NEW_CONSOLE)
+     logger.debug("exiting")
+     sys.exit(0)


+ def install_update_restart(
+     src_dir, dst_dir, purge_dst_dir=False, exclude_from_purge=None, **kwargs
+ ):
+     # Custom install function with restart functionality
+     """
+     Installs update files using platform specific installation script. The
+     actual installation script copies the files and folders from `src_dir` to
+     `dst_dir`.
+     If `purge_dst_dir` is `True`, *ALL* files and folders are deleted from
+     `dst_dir` before copying.
+     **DANGER**:
+     ONLY use `purge_dst_dir=True` if your app is properly installed in its
+     own *separate* directory, such as %PROGRAMFILES%\MyApp.
+     DO NOT use `purge_dst_dir=True` if your app executable is running
+     directly from a folder that also contains unrelated files or folders,
+     such as the Desktop folder or the Downloads folder, because this
+     unrelated content would be then also be deleted.
+     Individual files and folders can be excluded from purge using e.g.
+         exclude_from_purge=['path\\to\\file1', r'"path to\file2"', ...]
+     If `purge_dst_dir` is `False`, the `exclude_from_purge` argument is
+     ignored.
+     """
+     if ON_WINDOWS:
+         _install_update = _install_update_win
+     elif ON_MAC:
+         _install_update = _install_update_mac
+     else:
+         raise RuntimeError("This platform is not supported.")
+     return _install_update(
+         src_dir=src_dir,
+         dst_dir=dst_dir,
+         purge_dst_dir=purge_dst_dir,
+         exclude_from_purge=exclude_from_purge,
+         **kwargs,
+     )


def update(pre: str):
    # Create update client
    client = Client(
        app_name=settings.APP_NAME,
        app_install_dir=settings.INSTALL_DIR,
        current_version=settings.APP_VERSION,
        metadata_dir=settings.METADATA_DIR,
        metadata_base_url=settings.METADATA_BASE_URL,
        target_dir=settings.TARGET_DIR,
        target_base_url=settings.TARGET_BASE_URL,
        refresh_required=False,
    )

    # Perform update
    if client.check_for_updates(pre=pre):
        client.download_and_apply_update(
            # WARNING: Be very careful with purge_dst_dir=True, because this
            # will delete *EVERYTHING* inside the app_install_dir, except
            # paths specified in exclude_from_purge. So, only use
            # purge_dst_dir=True if you are certain that your app_install_dir
            # does not contain any unrelated content.
            progress_hook=progress_hook,
            purge_dst_dir=False,
            exclude_from_purge=None,
            log_file_name="install.log",
+             install=install_update_restart,
        )


def main(cmd_args):
    # extract options from command line args
    pre_release_channel = cmd_args[0] if cmd_args else None  # 'a', 'b', or 'rc'

    # The app must ensure dirs exist
    for dir_path in [settings.INSTALL_DIR, settings.METADATA_DIR, settings.TARGET_DIR]:
        dir_path.mkdir(exist_ok=True, parents=True)

    # The app must be shipped with a trusted "root.json" metadata file,
    # which is created using the tufup.repo tools. The app must ensure
    # this file can be found in the specified metadata_dir. The root metadata
    # file lists all trusted keys and TUF roles.
    if not settings.TRUSTED_ROOT_DST.exists():
        shutil.copy(src=settings.TRUSTED_ROOT_SRC, dst=settings.TRUSTED_ROOT_DST)
        logger.info("Trusted root metadata copied to cache.")

    # Download and apply any available updates
    update(pre=pre_release_channel)

    # Do what the app is supposed to do
    print(f"Starting {settings.APP_NAME} {settings.APP_VERSION}...")
    ...
    print("Doing what the app is supposed to do...")
+     time.sleep(10)
    ...
    print("Done.")

@its-monotype
Copy link
Author

Or maybe even better call start command like that ↓?

exe_path=dst_dir.joinpath(settings.EXE_PATH.name) # settings.EXE_PATH=pathlib.Path(sys.executable).resolve()

@dennisvang
Copy link
Owner

dennisvang commented Sep 28, 2022

@its-monotype could you edit your code above, to show only the lines that were changed (plus a little context)? It's a bit difficult for me to distinguish without copying and doing a diff.

@its-monotype
Copy link
Author

@dennisvang I changed the code above so you can see what has been added.

In short, I just created a custom install_update_restart function that I pass to client.download_and_apply_update(install=install_update_restart) and copied its content just from your install_update function, but in the WIN_MOVE_FILES_BAT variable I added the start {exe_path} command at the end and exe_path equal to exe_path=dst_dir.joinpath(settings.EXE_PATH.name) (settings.EXE_PATH=pathlib.Path(sys.executable).resolve()).

@dennisvang
Copy link
Owner

dennisvang commented Sep 28, 2022

@its-monotype Thanks, now I see what you're trying to do.

You're on the right track, but your code can be simplified considerably:

If you're only running on windows, you can remove your current install_update_restart function, then rename your _install_update_win to install_update_restart.

Now, as you probably know exactly what you're going to need, you can strip unnecessary stuff from this function.

To give you an idea:

  • If you're going to use the default robocopy options, you can remove everything related to the robocopy_options_override.
  • If you know you're not going to need purge, you can remove everything related to that as well (except for kwargs in the function definition).
  • You can also fill out part of the WIN_MOVE_FILES_BAT template, e.g. write the default options instead of the {options} placeholder.
  • If you know you're not going to install as admin, you can remove that part as well. (or if you are going to install only as admin, you can remove the non-admin install option)
  • And so on and so forth.

@its-monotype
Copy link
Author

@dennisvang Thank you very much for your advice 😊

@dennisvang
Copy link
Owner

@its-monotype You're welcome. I realize this workaround is a bit cumbersome, so I'm going to try to make this easier, see issue linked above.

@dennisvang
Copy link
Owner

dennisvang commented Sep 29, 2022

@its-monotype By the way, I see you use start {exe_path} in your custom batch script.

To prevent issues with whitespace in paths, I would enclose the variable in double quotes like this:

start "{exe_path}"

EDIT: Sorry, that probably won't work because start will then interpret exe_path as a title for the command window...

@dennisvang
Copy link
Owner

dennisvang commented Sep 29, 2022

@its-monotype Sorry, see edit above.

I guess that should be something like:

start "<title for the new cmd window>" "{exe_path}"

otherwise it will simply open a command window with the path as title, instead of actually running the executable.

Or you could do something like this, but I'm not sure if that's overly complicated:

start cmd /c "{exe_path}"

Also see this for some examples.

@dennisvang
Copy link
Owner

@its-monotype A new release is now available: 0.4.4

This makes it possible to specify a custom batch script or batch template.

Your example code above would now reduce to this:

...

CUSTOM_BATCH_TEMPLATE = """@echo off
{log_lines}
echo Moving app files...
robocopy "{src_dir}" "{dst_dir}" {robocopy_options}
echo Done.
echo Starting app...
start "" "{exe_path}"
{delete_self}
"""
NEW_EXE_PATH = settings.INSTALL_DIR / settings.EXE_PATH.name
...

client.download_and_apply_update(
    progress_hook=progress_hook,
    purge_dst_dir=False,
    exclude_from_purge=None,
    log_file_name='install.log',
    batch_template=CUSTOM_BATCH_TEMPLATE,
    batch_template_extra_kwargs=dict(exe_path=NEW_EXE_PATH),
)

...

@its-monotype
Copy link
Author

@dennisvang This is great, thank you so much for adding this feature so quickly, I'm very glad that you are actively developing and improving tufup! 👍

@dennisvang
Copy link
Owner

@its-monotype Thanks for the kind words, and thanks for helping us test-drive tufup. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants