DudeNr33 / pyinstaller-versionfile

Create a windows versionfile from a simple YAML file that can be used by PyInstaller.
MIT License
43 stars 5 forks source link

Take a dictionary instead of a file #7

Closed aw-was-here closed 3 years ago

aw-was-here commented 3 years ago

It would be fantastic if there was a way to replace copyright. Given the limitations of the version field, it is useful to stuff that field with more information at build-time. Thanks!

DudeNr33 commented 3 years ago

Can you please give a concrete example of what you want to achieve? Copyright itself can be set in the YAML file with LegalCopyright. Do you want to set this value from the command line?

aw-was-here commented 3 years ago

I'm actually directly calling pyinstaller_versionfile.create_file_version.MetaData as an API so that I can set things programmatically when pyinstaller runs. But it just occurred to me that what I probably need to do is send an IO steam that contains my content rather than have a file on the side.

DudeNr33 commented 3 years ago

I will take a look at this - so for you it would be enough if the MetaData class would accept a file-like object instead of a filepath?

aw-was-here commented 3 years ago

With a bit more playing, with the 1.0.2 version, this hack demonstrates what I'm looking to do.

aw-was-here commented 3 years ago

So really: rather than taking a raw file or a stream, taking a structure would be much more flexible.

DudeNr33 commented 3 years ago

@aw-was-here I pushed a update to the develop branch. For issue #6 I wanted to separate the input of the metadata and creation of the output file anyway, and this should now also make what you want to do straightforward, as you can just create your own MetaData object directly.

Here would be a minimal example:

from pyinstaller_versionfile.metadata import MetaData
from pyinstaller_versionfile.writer import Writer

metadata = MetaData(version="1.2.3.4", legal_copyright="My Imaginary Company")  # all parameters are optional, just specify what you need
writer = Writer(metadata)
writer.render()
writer.save("versionfile.txt")

Would this solve your problem? Note: this new version would become a V2.0.0 and will also drop support for Python < 3.6.

aw-was-here commented 3 years ago

Would this solve your problem?

@DudeNr33 Just tried out the develop branch: yes! definitely! This change makes it much easier to incorporate into the build.

Two things probably worth mentioning:

1) It looks like version validation/fixing isn't actually happening with < 4 digits. I'd expect either a ValueError exception or for the code to just automatically fix version if it isn't 4 decimal places. e.g.:

metadata = MetaData(version='1.2.3', ...)

ends up being

filevers=(1,2,3),

which is obviously wrong.

2) This usage feels awkward, given I'm not sure what else I'd use the Writer object for:

writer = Writer(metadata)
writer.render()
writer.save("versionfile.txt")

Should save() just call render()? Should render() return the rendered text as well? Or perhaps you have other plans already and it will make more sense later. :+1:

Note: this new version would become a V2.0.0 and will also drop support for Python < 3.6.

That's fine for me. I've already got code that requires 3.8 in use.

Thanks!

DudeNr33 commented 3 years ago

Thank you for your feedback!

Ad 1) The MetaData class has two additional methods, that you may call from your calling code:

I considered calling both methods directly in the __init__ method of the class. But I thought that if somebody imports and uses the code directly, he would probably also like a bit more control over what is happening, so I decided against it.

Ad 2) render() takes care of rendering the output and handling rendering errors. save() takes care of the saving the file and handling I/O errors. I found it cleaner to separate those concerns into individual methods.

I think both issues could be addressed by providing a functional API in the __init__.py of the package, which would essentially implement the logic currently in main.py:main() for the two cases of file-based input and dictionary (or keyword arguments) input.

# __init__.py

def create_versionfile(version, company_name, ..., outfile):
    metadata = MetaData(version, company_name, ...)
    metadata.validate()
    metadata.sanitize()
    writer = Writer(metadata)
    writer.render()
    writer.save(outfile)

def create_versionfile_from_file(infile, outfile, version=None):
    metadata = MetaData.from_file(infile)
    metadata.validate()
    if version:
        metadata.set_version(version)
    metadata.sanitize()
    writer = Writer(metadata)
    writer.render()
    writer.save(outfile)

This would at least save you from calling all methods yourself in the correct order. Having a second opinion is always great, so fell free to let me know what you think.

aw-was-here commented 3 years ago

That all looks fantastic. Combined with pyinstaller taking the version file as a parameter these changes make it all very easy to do now.

Thanks!

DudeNr33 commented 3 years ago

V2.0.0 is now available on PyPI. Check the updated readme for the final function signature. 👍

aw-was-here commented 3 years ago

Thanks!