Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Windows Build #179

Merged
merged 88 commits into from
Jul 2, 2024
Merged

Fix Windows Build #179

merged 88 commits into from
Jul 2, 2024

Conversation

ayjayt
Copy link
Collaborator

@ayjayt ayjayt commented Apr 22, 2024

Win build is fixed for Chromium Stable 88- the last one plotly released.

Highlighted wins:

  • No longer a 53gb download, now a 20gb download.
  • Windows builds!

What's Next

  • DONE: I need to get it up somewhere on pypi so I can ship my own
  • Find out from Plolty and Jon if they're interested in rebooting CI, providing commentary, assisting in testing etc.

"Urgent" Technical Issues

- Migrate Tooling

Use make, bash, rsync for everything. Move away from scripts except for CI. Unify interface between platforms, factor out common code, and make interface should be:

make set_version # choose chromium version, choose platform, everything else is auto
make fetch_tools # git clone depot_tools
make init_tools #  look at note in fetch_chromium.ps1, info changes based on version/platform?
make sync # runs gclient
make gen_meta # CREDITS, license.py, version, README, etc
make patch # runs patches based on versions
make config_build # appends kaleido instructions to `BUILD.gn`, runs `gn gen out`
make compile # runs ninja
make extract_build # changes based on version! see next point as well. also does mathjax, etc.
make npm # does npm stuff
make javascript # copies npm stuff
make wheel # duh
# fun fact, the zip part is unecessary because wheels actually are `.zip` files of python packages renamed to `.whl`...

Reorganizing the repository would come along with this. It's really only a days labor, especially with this outline.

The problem is 'version.py' constantly changes trivial code and forces recompile of 5k objects. Mal!

For the CI, it's a simple as skipping make set_version which really just sets environmental variables and then running make all. Boom!

- Determine run-time dependencies

We have this annoying issue where even ldd won't show us which dll/dynlib/so are needed in the wheel because they appear to be linked dynamically. So for every version, given that outputs and chromium needs change, we need to determine which outputs to our compile actually need to come with us (LOTS do not). The solution for this is as follows:

  1. Python is a shell for kaleido, and kaleido and chromium should optionally report all errors back to the python.
  2. Test kaleido against ALL mocks.
  3. Tests kick back if chromium throws an error to python.

- Remove dependency on base::... just to reduce maintenance

- Update Chromium API... no other option, they 86'ed it.

Longer Discussion

What I did:

  1. We don't download the entire chromium repo anymore, just the revevant tag. 30GB in savings.

  2. Errors were fixed in power shell scripts (using what looked like bash)

  3. Power shell scripts now fail loudly instead of pretending to succeed.

  4. Exhaustive build documentation, but to the point and separate from user documentation (and also divided by platform).

  5. Got Chromium 88 to build for windows, had to write two patches.

  6. Got Chromium 108 to build, but has runtime errors.

Unfortunately, the API used in kaleido was officially removed after version 108, and had been degrading up until that point. Migrating to the current API is a bit of a job.

Also, we're relying on chromiums base:: namespace, which they don't recommend. It is unstable, but I did update it to work to get version 108 to build. Version 88 uses old version of base::. A long-lasting solution would actually minimize exposure to chromium APIs, ironically.

All this so my users can make videos like this:

all.mp4

Pretty, aint it?

@ayjayt
Copy link
Collaborator Author

ayjayt commented Apr 22, 2024

Interestingly, kaleido==0.2.1 works on some windows machines over here, but not mine. So far my fix works on all ours.

Here is mine: pip install --index-url https://test.pypi.org/simple/ kaleidowinfix2

Normally don't recommend installing exe's from randos.

@jonmmease, just tagging you for visibility. thanks for getting all this started.

@archmoj could you tag anyone who might be interested or benefit from this at plotly? i would like to coordinate a re-release and i have no experience with circle ci.

@Coding-with-Adam tagging you because there is a bunch of community questions about this, tons on github and a couple in the forums. i know you're waiting for c2m but this was blocking 3 people over here so I had to do it.

@ndrezn
Copy link
Member

ndrezn commented May 2, 2024

Hi @ayjayt! Thanks for this contribution! We're taking a look internally and looking to set time to review this PR.

@ayjayt
Copy link
Collaborator Author

ayjayt commented May 2, 2024 via email

@gvwilson
Copy link
Collaborator

thanks very much for this @ayjayt - any chance you could spare a few minutes next week to chat online? cheers - @gvwilson

@gvwilson gvwilson self-assigned this May 27, 2024
@ayjayt
Copy link
Collaborator Author

ayjayt commented May 27, 2024

Sure @gvwilson, things are pretty wild over here on my end right now but I can make time for a chat. [email protected] or however you'd like.

Copy link
Collaborator

@gvwilson gvwilson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me - thank you

@ayjayt ayjayt marked this pull request as ready for review July 2, 2024 18:01
@ayjayt ayjayt merged commit 056642f into plotly:master Jul 2, 2024
@disberd
Copy link

disberd commented Aug 6, 2024

@gvwilson do you have an idea on whether this PR can be used to create a new release in the hope that problems with kaleido 0.2.1 on windows disappear?

@gvwilson
Copy link
Collaborator

gvwilson commented Aug 6, 2024

Hi @disberd - I hope we'll have news in a couple of weeks about the state of Kaleido - still experimenting.

@RoyiAvital
Copy link

Hi, Many users in the Julia eco system have issues with the current release.
There is a chance this will fix the issues. Any chance making a release out of it?

It will benefit many.

@ayjayt
Copy link
Collaborator Author

ayjayt commented Sep 13, 2024

Hi Royi,

This development process is wrapping up and I am predicting a release very soon.

But I AFAIK no julia adaption has been written yet for the python side of Kaleido. Is there one I'm not aware of?

Thanks,

@awderh
Copy link

awderh commented Oct 15, 2024

Hi @gvwilson - congrats on the EM position. Any updates on the rewrite?

I want to take a look at Windows hanging problem but I don't want it to go to waste or duplicate effort; noticed you had made similar remarks on other threads. Maybe you can spin-up another issue to consolidate the discussion?

@ayjayt
Copy link
Collaborator Author

ayjayt commented Oct 15, 2024

Hi @awderh, which issue are you referring to? This fixes the windows build. There's entire set of PR to update kaleido to use the new version of chromium (at the time, now 3 months behind) to fix the build process, etc.

But there are newer technologies that don't necessarily require us shipping chromium (>120MB) with the package, so we have an imminent release of that.

@ayjayt
Copy link
Collaborator Author

ayjayt commented Oct 15, 2024

Hi Royi,

This development process is wrapping up and I am predicting a release very soon.

But I AFAIK no julia adaption has been written yet for the python side of Kaleido. Is there one I'm not aware of?

Thanks,

@RoyiAvital sorry I meant to @ you in my reply to you.

@awderh
Copy link

awderh commented Oct 15, 2024

Hi @awderh, which issue are you referring to? This fixes the windows build. There's entire set of PR to update kaleido to use the new version of chromium (at the time, now 3 months behind) to fix the build process, etc.

But there are newer technologies that don't necessarily require us shipping chromium (>120MB) with the package, so we have an imminent release of that.

My original message to @gvwilson was regarding the hanging issue on Windows that @disberd was also referencing. See #134

@ayjayt
Copy link
Collaborator Author

ayjayt commented Oct 15, 2024

Hi @awderh,

I believe I fixed that issue in this PR, but if not, it would definitely be in branch: https://github.com/plotly/Kaleido/tree/andrew/windows_restructure. Neither this PR nor that branch have a release, Chromium's headless size has grown larger than what pypi allows by default, and their support is very backlogged.

The next release, however, will be based on #206 which is dependent on a package that is currently private (hoping that changes today 🤞 but pypi pushes have extra security checks > github pushes.

edit: I can @ you if you'd like to try it as soon as its released on github and then again on pypi

@awderh
Copy link

awderh commented Oct 15, 2024

Hi @ayjayt - awesome! Yes, please do ping me and I'll try it asap

@ayjayt
Copy link
Collaborator Author

ayjayt commented Oct 17, 2024

@awderh there is a PR up for a new take on kaleido:

a. it requires a system install of chrome or chromium

If you install that version of kaleido it should work for many applications, although there are some advanced features (mapbox changes, for example) that would break.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants