allow commandline arguments to be passed through to cmake

[ssteinbach]

Sometimes its helpful to be able to pass command line arguments through setup.py to cmake. This change appends the CMAKE_ARGS variable to the flags passed to cmake. It can be used like this:

env CMAKE_ARGS="-DCMAKE_VAR_1=VALUE1 -DCMAKE_VAR_2=VALUE2" python setup.py build
or
env CMAKE_ARGS="-DCMAKE_VAR=VALUE1 -DCMAKE_VAR_2=VALUE2" pip install .

TODO:

• Add a note about the environment variable to the readthedocs developer QuickStart
• Add a note about the environment variable to the README.md

[codecov-commenter]

Codecov Report

Merging #1006 (1ca3f09) into master (aada787) will increase coverage by 3.54%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1006      +/-   ##
==========================================
+ Coverage   82.78%   86.32%   +3.54%     
==========================================
  Files          75      191     +116     
  Lines        3306    19004   +15698     
  Branches        0     2105    +2105     
==========================================
+ Hits         2737    16406   +13669     
- Misses        569     2052    +1483     
- Partials        0      546     +546     

Flag	Coverage Δ
unittests	86.32% <ø> (+3.54%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...pentimelineio/opentimelineio/adapters/otio_json.py	100.00% <0.00%> (ø)
...y-opentimelineio/opentimelineio/schema/__init__.py	100.00% <0.00%> (ø)
tests/baseline_reader.py	100.00% <0.00%> (ø)
...elineio_contrib/adapters/aaf_adapter/aaf_writer.py	96.07% <0.00%> (ø)
...pentimelineio/opentimelineio/schemadef/__init__.py	100.00% <0.00%> (ø)
src/py-opentimelineio/opentimelineio/__init__.py	100.00% <0.00%> (ø)
tests/test_image_sequence_reference.py	97.59% <0.00%> (ø)
...-opentimelineio/opentimelineio/adapters/adapter.py	87.12% <0.00%> (ø)
.../opentimelineio/schema/image_sequence_reference.py	100.00% <0.00%> (ø)
tests/test_filter_algorithms.py	98.89% <0.00%> (ø)
... and 106 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aada787...1ca3f09. Read the comment docs.

[reinecke]

Nice add!
Something we might think about is finding a place to document these so they don't stay in the realm of tribal knowledge.

[ssteinbach]

Yes, although part of the reason I'd prefer to break up the build systems is so that it's possible to communicate directly with cmake using standard cmake tools.

Not having to document these weird pass throughs the setup.py would be better... They aren't discoverable and are pretty non standard as far as I've seen.

[JeanChristopheMorinPerso]

@ssteinbach There is ways to expose custom arguments.

For example we could have:

class OTIO_build_ext(setuptools.command.build_ext.build_ext):

    user_options = setuptools.command.build_ext.build_ext.user_options + [
        ('cmake=', None, 'JC custom extension')
    ]

    def initialize_options(self):
        setuptools.command.build_ext.build_ext.initialize_options(self)
        self.cmake = None  # Really important. If you don't do this, the option will not be available

Running python setup.py build_ext --cmake="-DCMAKE_VAR_1=something -DCMAKE_VAR_2=else" would make the variables accessible in the self.cmake variable.

the --cmake is discoverable when using python setup.py build_ext --help.

Not sure if that would be an acceptable solution.

The downside is that I don't think we can pass this argument to pip in any way. The usual --global-option/--install-option/--build-option don't seem to work.

[ssteinbach]

Yeah... I think in this case we need to pass these options through pip since that is the way that wheels are to be built. But that is a great suggestion... pip doesn't have functionality like that?

[JeanChristopheMorinPerso]

I think in this case we need to pass these options through pip since that is the way that wheels are to be built.

You can create a wheel like this:

python setup.py build_ext
python setup.py bdist_wheel

and because build_ext is run before bdist_wheel, there will only be a single compilation and bdist_wheel just takes the existing artifacts and creates a wheel with them.

pip doesn't have functionality like that?

Pip has some flags, like --global-option, --install-option. It's not too clear what the global flag does. As for the install flag, it passes the flag to the setup.py install command.

I've tried them and I'm still not too sure how everything works with these flags. There is also the fact that if you specify flags like these, they will apply to all dependencies being installed... Which means the cmake python package will be completely rebuilt. One thing I noticed is that if you pip install ., our setup.py will be invoked with setup.py bdist_wheel. But the thing is we don't have a custom class/command for bdist_wheel. Which means setuptools invokes/uses our custom build_ext class/command somehow when bdist_wheel is run. I'm not sure how it's done but I have a feeling that by understanding this it would probably answer if it's doable to do that with pip.

[ssteinbach]

@JeanChristopheMorinPerso I did another pass at this w/ a rebase. Do you still have concerns over this approach? I think we need this for internal build system stuff.

[JeanChristopheMorinPerso]

@ssteinbach I'm good with the approach. My concern was just that I am under the impression we can do this already but haven't figured out yet how. If we later find a cleaner (read "more native") way we can always create a new PR.

So I guess it's alright to merge.

[ssteinbach]

Nice add!
Something we might think about is finding a place to document these so they don't stay in the realm of tribal knowledge.

@reinecke I added some notes to the README and to the Quickstart.md. Any notes on that or does that look good?

[meshula]

Maybe a slight tweak to say which shell (bash?) the example env command works with? It is probably something else for cmd.exe, for example :)

[ssteinbach]

sure.

[ssteinbach]

@meshula how do these changes sound?

[meshula]

These changes are good and address Eric's request IMO. I was just suggesting that the env statement could use an annotation around line 134 that instead of saying "Example:" says "Example for a unix style shell" or whatever. There's a lot of Windows OTIO users, and it would be nice to provide that extra hint that their mileage might vary.