Allow overriding configuration options

[brechtm]

I want to override the application version, so I refactored the main() function into main() and run_installer_builder().

[brechtm]

Please consider releasing 0.12 if you accept this PR.

[brechtm]

Just a gentle reminder to take a look at this PR if you can find the time. Thanks!

[takluyver]

Hi, sorry it's taken me a while to get to this!

So you're using a config file, but overriding some of the options using a custom Python script? What's the motivation for this? Are there things you can't set from the config file? Settings that you want to choose programmatically? Is this for building your own installer, or are you making a tool wrapping pynsist for other people to use? (I'm not trying to say there's anything wrong with what you're doing, I just want to understand how you're using Pynsist)

My initial reaction was that putting together the existing APIs should provide enough flexibility, but looking at the bits you've pulled out, I see there are several lines of code there, and it doesn't all look like stuff I'd encourage people to use as public API at present. So I can see that some simplification is necessary.

If we do do it like this, I'm not sold on the name - run_installer_builder is doing quite a bit more than InstallerBuilder.run(). But I'd like to come up with a better structure. I prefer the Python API to have two main steps: parsing and validating config, and using that config to actually build an installer. If the config is an object that your code retrieves and passes back to InstallerBuilder, it's easy to override options by modifying the object before passing it in. Does that sound reasonable?

os.chdir() is a tricky one. I don't like library functions changing the cwd, and if I intended pynsist to be used primarily as a library, I'd put in the effort to make it independent of the cwd. But for something that I offer primarily as a CLI, it's a nice convenience to just set the cwd and let it take care of relative paths.

[takluyver]

One more question (sorry!) for my understanding: if you're running Pynsist from your own code, why do you want to load a config file, as opposed to writing all of the options in your script? The answer may simply be "seems neater that way", but I'm interested to hear your thoughts.

[brechtm]

Me too, I'm sorry for the delay in getting back to you. I've been taking a little break...

And don't worry, I encourage a thorough review of this PR (or any PR)! I basically just made it work for my use case without paying much attention to the bigger picture, I have to admit.

I basically want to set the version to whatever the version of my package is. For a development version, this includes the git commit hash, for example. I do this by installing my package into pynsist_pkgs and then use pkg_resources.WorkingSet to get the version number:

working_set = WorkingSet(['pynsist_pkgs'])
rinohtype_version = working_set.by_key['rinohtype'].version
run_installer_builder('wininst.cfg', version=rinohtype_version)

If the config is an object that your code retrieves and passes back to InstallerBuilder, it's easy to override options by modifying the object before passing it in. Does that sound reasonable?

That would be perfect!

os.chdir() is a tricky one. I don't like library functions changing the cwd, and if I intended pynsist to be used primarily as a library, I'd put in the effort to make it independent of the cwd. But for something that I offer primarily as a CLI, it's a nice convenience to just set the cwd and let it take care of relative paths.

I agree that it would be best to avoid this, but I've made the minimum of changes to make it possible to override the configuration options. It can indeed be tricky to make things independent of the CWD. Not an ideal solution, but you could make use of a context manager that restored the CWD at the end. This might be good enough for now.

One more question (sorry!) for my understanding: if you're running Pynsist from your own code, why do you want to load a config file, as opposed to writing all of the options in your script? The answer may simply be "seems neater that way", but I'm interested to hear your thoughts.

I guess I could do that, but it's indeed neater that way since I only need to override the version.