search

Origen-SDK / o2

MIT License
4 stars 0 forks source link

Python Package Build #116

Closed ginty closed 3 years ago

ginty commented 3 years ago

This adds the ability to build and publish the Origen Python package and adds a new origen build command which will make developer's lives easier. The Python packages are published here - https://pypi.org/search/?q=origen

I had hoped to build just one 'origen' Python package that contained both the native Python code and the compiled extension, but the build tools did not allow that and we have to have two - origen and origen_pyapi. In practice, this is just an internal detail as each version of origen will depend on the corresponding origen_pyapi of the same version and this detail will be invisible to users - i.e. they will just depend on origen within their apps.

This adds an origen build command which has switches for changing the version, publishing to https://pypi.org and of course building Origen. Most of these switches will only be used by the CI scripts and as they are already added anyway I won't go into them here. However, the plan is that when we push a tag Travis should build and publish the Python packages for all platforms and Python versions automatically. I haven't fully run that right through yet, but will make a tag after merging this and fix any bugs.

For developers, the commands you want to run (and which can be run from anywhere within the o2 workspace):

origen build    # Build pyapi and plug it into the example app

origen build --cli   # Build the CLI

When setting this up I noticed that the new origen package could not be imported into an ad-hoc Python file - it crashed looking for an Origen app. So I've added a new test area at test_apps/python_no_app to test this scenario, though it doesn't do much yet.

To keep the top-level directory clean I moved the existing example app to test_apps/python_app.

The main README has been updated for this new system, you should review these instructions when transitioning your existing workspace to this - particularly see point (5) about some Python packages that are required to be present.

ginty commented 3 years ago

Hi @coreyeng, happy to let you handle the merge if you don't mind, thanks!

Before merging this, all the Windows regressions are failing, is this the same as you saw:

image

coreyeng commented 3 years ago

Yes, I encountered that as well. I have a note on this in #114:

Lastly, and kind of unrelated to this, but some of the crates won't build in Windows with the gnu compiler. I switched the Travis script back to using msvc, which is what Rust has as the official installation, so I think this config is more valid than the gnu one.

I couldn't tell if it was complaining about the actual core or the cfg-if crate but either way, it looks like Windows GNU support was dropped. A screenshot from the latest cfg-if doesn't show it as supported.

image

If it is the core, the Rust guides say to use msvc anyway.

Actually, I think this discussion is even more applicable here since, in theory, Travis will now build the executables and Windows users can just download them straight away. Now, I'm not sure if those executables will still require msvc bindings but if they do then I'd imagine they would also require GNU bindings if we were to build with that so Windows users will end up installing one of the two anyway - in which case I think it should be msvc based on the Rust documentation.

pderouen commented 3 years ago

Just a heads up, when using msvc you should add an environment variable (RUSTFLAGS) before building in order to get a static linked binary.

RUSTFLAGS = '-C target-feature=+crt-static'
pderouen commented 3 years ago

Actually, I think this discussion is even more applicable here since, in theory, Travis will now build the executables and Windows users can just download them straight away. Now, I'm not sure if those executables will still require msvc bindings but if they do then I'd imagine they would also require GNU bindings if we were to build with that so Windows users will end up installing one of the two anyway - in which case I think it should be msvc based on the Rust documentation.

If you build with static links to msvc, the resulting binary will run on other windows machines without any additional installations needed. I have tested this out with my standalone internal tool.

coreyeng commented 3 years ago

Thanks @pderouen! So it should really only be an issue for developers. I still think going with msvc is the way though, per the Rust docs, and should be transparent to the users anyway.

@ginty, I have this fixed on my branch so if you want to merge this with it failing, I'll take of it.