cloudflare / stpyv8

Python 3 and JavaScript interoperability. Successor To PyV8 (https://github.com/flier/pyv8)
Apache License 2.0
440 stars 39 forks source link

Use latest Boost version across all Windows builds #85

Closed devm18426 closed 9 months ago

devm18426 commented 1 year ago

Consider waiting until https://github.com/MarkusJx/prebuilt-boost/issues/6 is resolved so that we may enjoy the latest version of Boost (1.83.0)

devm18426 commented 1 year ago

@buffer It turns out that install-boost doesn't provide Boost binaries that can be statically linked with on Windows. I've been discussing this topic with the maintainer here. He recommends linking dynamically with Boost, which would probably require packaging the Boost lib in the STPyV8 wheel file. Alternatively, this package could require the user preinstall the appropriate Boost binaries on their machine (less elegant option in my opinion).

Have you considered either of these approaches?

It would appear that the maintainer of install-boost is open to the possibility of supporting statically-linkable binaries, with no guarantees. I think that would be best but I'm not sure if I should ask him to look into it.

Otherwise, there's always the option of staying with the current flow of compiling Boost as part of the build/deployment process. This option isn't bad at all, just adds some overhead and minor maintenance.

What are your thoughts?

MarkusJx commented 1 year ago

Hi, I'm the author of install-boost, not sure if that matters but here you go.

Since you've linked this issue I should probably clarify some things in order to save all of us some time and unnecessary work, as there seems to be a misunderstanding:

I hope this helps with your issue. Also, if you are already using the /MD link option, your issue is probably related to something else as boost seems to do weird things regarding its python libraries. Here is another discussion I've had a while ago also regarding the (static) python libraries, maybe there's a solution in there: https://github.com/MarkusJx/install-boost/discussions/25#discussioncomment-4016912.

buffer commented 1 year ago

Thanks @devm18426 and @MarkusJx for taking care of this. I have noticed that we are currently compiling with the option /MT so I would try switching to the option /MD as per @MarkusJx suggestions and check if that's enough to fix the issue. If not we should probably think about a different approach

devm18426 commented 1 year ago

@buffer is this change still necessary? Following your latest changes to the workflows? If so, I won't be able to get back on it in the next while.

buffer commented 1 year ago

@devm18426 I think the change is still required but I changed the workflow files recently. And I will take care of porting your changes to the new files if you're busy. I think we should move to use your approach for all the workflows so to be totally independent from the boost library installed on the system, if any. This will reasonably be a very important step to be able to distribute STPyV8 through PyPI. Which could be considered the final goal IMHO

buffer commented 1 year ago

@devm18426 FYI I was able to modify the OSX debug workflow [1] which now installs Boost using [2] and statically link it to the generated wheel. I tested the wheel locally and boost and boost-python local packages are not required anymore. The PR is still far from perfect but I think that could be a nice approach to distribute STPyV8 Python through PyPi. Still have to think about how to distribute the icudtl.dat file but that's another story.

[1] https://github.com/cloudflare/stpyv8/pull/90 [2] https://github.com/MarkusJx/install-boost

buffer commented 9 months ago

Closing this PR after https://github.com/cloudflare/stpyv8/pull/98 was merged