takluyver / pynsist

Build Windows installers for Python applications
https://pynsist.readthedocs.io/
Other
896 stars 119 forks source link

Modified pjoin command to ensure path is formatted correctly #154

Closed JoshMayberry closed 6 years ago

JoshMayberry commented 6 years ago

This will handle user paths that contain /, \, and \\. Included files like "$OUTDIR/docs/code/subfolder" will not collapse into $OUTDIR/docscodesubfolder either.

takluyver commented 6 years ago

Thanks! Before we get into reviewing the changes, however, can you describe the problem that you've encountered - what path did you use and where, and what result did you see?

JoshMayberry commented 6 years ago

Sure thing. I used the following code:

packages = ["GUI_Maker", "API_Database", "API_Excel", "API_Com"]
excludes = ['_ssl', 'pyreadline', 'doctest', 'optparse', 'pickle', 'calendar', 'pdb', 'unitest', 'numpy', 'tkinter']
includes = []
build_dir = 'C:/Users/jmayberry/Desktop/materialTrackerProgram'
license_file = None
py_version = '3.6.4'
py_bitness = 32
data_files = [('controller.py', '$INSTDIR/'), ('resources', '$INSTDIR/'), ('data.db', '$INSTDIR/'), ('docs', '$INSTDIR/'), ('runMe.py', '$INSTDIR/docs/code'), ('setup.py', '$INSTDIR/docs/code'), ('__init__.py', '$INSTDIR/docs/code'), ('controller.py', '$INSTDIR/docs/code'), ('test_Material_Tracker.py', '$INSTDIR/docs/code'), ('H:/Python/modules/API_COM/controller.py', '$INSTDIR/docs/code/modules/API_COM'), ('H:/Python/modules/API_Database/controller.py', '$INSTDIR/docs/code/modules/API_Database'), ('H:/Python/modules/GUI_Maker/controller.py', '$INSTDIR/docs/code/modules/GUI_Maker'), ('H:/Python/modules/GUI_Maker/Splash.py', '$INSTDIR/docs/code/modules/GUI_Maker'), ('H:/Python/modules/GUI_Maker/test_GUI_Maker.py', '$INSTDIR/docs/code/modules/GUI_Maker'), ('H:/Python/modules/GUI_Maker/docs', '$INSTDIR/docs/code/modules/GUI_Maker'), ('H:/Python/modules/GUI_Maker/examples', '$INSTDIR/docs/code/modules/GUI_Maker')]
script = 'runMe.py'
icon = 'resources/startIcon.ico'
publisherName = None
installerName = 'myInstaller'
name = "materialTracker"
version = "1.0.0"
include_cmd = True

if (os.path.exists(build_dir)):
    def onerror(function, path, exc_info):
        """An Error handler for shutil.rmtree.
        Modified code from Justin Peel on https://stackoverflow.com/questions/2656322/shutil-rmtree-fails-on-windows-with-access-is-denied
        """
        if (not os.access(path, os.W_OK)):
            os.chmod(path, stat.S_IWUSR)
            function(path)
        else:
            raise
    shutil.rmtree(build_dir, ignore_errors=False, onerror=onerror)

builder = nsist.InstallerBuilder(name,
    version,
    {name: 
        {"script": script,
        "console": include_cmd,
        "icon": icon
        }
    },
    publisher = None, #publisherName,
    icon = icon,
    packages = packages + includes,
    extra_files = data_files,
    py_version = py_version,
    py_bitness = py_bitness,
    build_dir = build_dir,
    installer_name = None, #installerName,
    exclude = excludes,
    license_file = license_file,
    # extra_installers = extra_installers,

    nsi_template = None,
    extra_wheel_sources = None,
    pypi_wheel_reqs = None,
    py_format = 'bundled',
    inc_msvcrt = True,
    commands = None,
    )

builder.run()

And when I run the installer, the following are the paths that are created for the extra_files:

C:\Users\jmayberry\AppData\Local\materialTrackerdocs
C:\Users\jmayberry\AppData\Local\materialTrackerdocscode
C:\Users\jmayberry\AppData\Local\materialTrackerdocscodemodulesAPI_COM
C:\Users\jmayberry\AppData\Local\materialTrackerdocscodemodulesAPI_Database
C:\Users\jmayberry\AppData\Local\materialTrackerdocscodemodulesGUI_Maker
C:\Users\jmayberry\AppData\Local\materialTrackerresources

Whereas, the file structure should be the following:

C:\Users\jmayberry\AppData\Local\materialTracker\docs
C:\Users\jmayberry\AppData\Local\materialTracker\docs\code
C:\Users\jmayberry\AppData\Local\materialTracker\docs\code\modules\API_COM
C:\Users\jmayberry\AppData\Local\materialTracker\docs\code\modules\API_Database
C:\Users\jmayberry\AppData\Local\materialTracker\docs\code\modules\GUI_Maker
C:\Users\jmayberry\AppData\Local\materialTracker\resources

With the changes I have made, the correct file structure is created because the generated file installer.nsi does not say things like:

SetOutPath "$INSTDIR/docs/code/modules/GUI_Maker\docs"

But instead says things like:

SetOutPath "$INSTDIR\\docs\\code\\modules\\GUI_Maker\\docs"

This is what I meant when I said: Included files like "$OUTDIR/docs/code/subfolder" will not collapse into $OUTDIR/docscodesubfolder

takluyver commented 6 years ago

Thanks, that makes sense.

I'd like to move the normalisation closer to where the paths are passed in, rather than doing it in the template. Maybe the copy_extra_files() function would be a good place for it.

I think the standard Python function ntpath.normpath should do the right thing with slashes. If not, we could add a function in nsist.util alongside the normalize_path function already there.

JoshMayberry commented 6 years ago

Sounds good. You understand your program much better than I do.

takluyver commented 6 years ago

I'm still happy for you to implement the changes, if you're interested. I have an advantage in understanding the code I wrote :wink:, but it's valuable to have more people understanding it.

If you don't have time any more, let me know, and I'll try to get round to it at some point.

JoshMayberry commented 6 years ago

At the moment, I don't have the time to do it. My boss let me figure out how to make it work, because we needed something that works. Now that we have a working solution (although, not the best solution), we need to move forward with other tasks. :( I'm sorry.

takluyver commented 6 years ago

PR #158 is my attempt to fix this - do you have time to check if that works for you?

JoshMayberry commented 6 years ago

I just tried it out. PR #158 fixes the path issue.

takluyver commented 6 years ago

Thanks. Next time I get a chance to work on this, I'll try to figure out something for #156.