Skip to content

Issues on Windows (and other platforms), is shell=True required? #13

@IanGallacher

Description

@IanGallacher

I believe that the current apkdiff.py is broken on windows (tested on 64 bit win10 with 64 bit python 3.11). I always get the following output:

C:\Users\dev\Downloads\test_dir_for_safety>python C:\Users\dev\Documents\GitHub\apkdiff\apkdiff.py C:\Users\dev\Downloads\file1.apk  C:\Users\dev\Downloads\file2.apk

                         apktool

Running apktool against '�[94mC:\Users\dev\Downloads\file1.apk�[0m'
Traceback (most recent call last):
  File "C:\Users\dev\Documents\GitHub\apkdiff\apkdiff.py", line 194, in <module>
    main()
  File "C:\Users\dev\Documents\GitHub\apkdiff\apkdiff.py", line 61, in main
    apktoolit(args.apk1, temp1)
  File "C:\Users\dev\Documents\GitHub\apkdiff\apkdiff.py", line 81, in apktoolit
    call(["apktool", "d", "-r", "-f", "-o", dir, file], stdout=open(os.devnull, 'w'), stderr=STDOUT)
  File "C:\Users\dev\AppData\Local\Programs\Python\Python311\Lib\subprocess.py", line 389, in call
    with Popen(*popenargs, **kwargs) as p:
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\dev\AppData\Local\Programs\Python\Python311\Lib\subprocess.py", line 1024, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "C:\Users\dev\AppData\Local\Programs\Python\Python311\Lib\subprocess.py", line 1493, in _execute_child
    hp, ht, pid, tid = _winapi.CreateProcess(executable, args,
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
FileNotFoundError: [WinError 2] The system cannot find the file specified

I believe I'm able to fix the issue if I change what is currently line 81 from

call(["apktool", "d", "-r", "-f", "-o", dir, file], stdout=open(os.devnull, 'w'), stderr=STDOUT)

to

call(["apktool", "d", "-r", "-f", "-o", dir, file], stdout=open(os.devnull, 'w'), stderr=STDOUT, shell=True)

or

call(["cmd", "/c", "apktool", "d", "-r", "-f", "-o", dir, file], stdout=open(os.devnull, 'w'), stderr=STDOUT)

Obviously, the "cmd", "/c" is windows-exclusive and isn't cross-platform. However, shell=True might be safe to add.

I looked at making a PR to add shell=True, but I was surprised to see that there are already a few PRs and issues that dabble with that have dabbled with shell=True.

It sounds like shell=True is required for Ubuntu according to #4, and in my experience it is also required on Windows.

PR #9 mentions that breaks "most other operating systems".

I'm raising this issue to see if it is safe to re-add shell=True. If you know of any issues or specific problematic operating systems with adding shell=True, please call them out.

This might not be the ideal fix, but I imagine it would be good if we got the tool working out of the box on Windows.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions