Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Download jquery during build.#11246
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
This failed most tests on the boilerplate issue. OTOH, it is also failing py 3.5... |
Rebased and fixed py3.5 issues. |
dc7bcb1 to090a743Compare
timhoffm left a comment• edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
There is currently no way to replace the download by a local version (e.g. via an environment variable as suggested in#11199). I think we should not force an internet connection for builds.
Would it make sense to define a local path within the checkout, e.g..cache/jquery-ui-1.12.1 as the source? People can then manually copy it there if they don't want a download. Otherwise, we download and extract it there and subsequently use this version.
Further advantage: This also reduces the downloads a bit, because we have just one download per checked out repo, not per install/develop.
| # Note: When bumping the jquery-ui version, also update the versions in | ||
| # single_figure.html and all_figures.html. | ||
| url = "https://jqueryui.com/resources/download/jquery-ui-1.12.1.zip" | ||
| if not os.path.exists(os.path.join(dest, "jquery-ui-1.12.1")): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Better not to hard-code the subdirectory. This can easily go wrong in a future update.
targetdir = Path(dest) / Path(url.split('/')[-1]).stemif not targetdir.exists():There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I think forgetting to update the HTML template is a much worse risk. Plus see below re: py2 compatibility.
| # single_figure.html and all_figures.html. | ||
| url = "https://jqueryui.com/resources/download/jquery-ui-1.12.1.zip" | ||
| if not os.path.exists(os.path.join(dest, "jquery-ui-1.12.1")): | ||
| try: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
os.makedirs(dest, exist_ok=True)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Written to be py2 compatible to simplify the backport (we can always update it later).
@tacaswell said he would handle the caching :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Approved. But should only be merged, when there is also caching in place.
I'm confused - should we merge this and close#11768? |
No,@tacaswell wants to do some more work on it first. |
@sandrotosi You may need to download jquery-ui as part of the debian build process now (as iirc you run the builds without a network connection). |
BLD: devDownload jquery during build Conflicts:examples/user_interfaces/embedding_webagg_sgskip.pylib/matplotlib/backends/web_backend/all_figures.htmllib/matplotlib/backends/web_backend/single_figure.htmlsetup.pysetupext.py
BLD: devDownload jquery during build
…-v3.0.xMerge pull request#11246 from anntzer/download-jquery
Well, sorry, my py2fu was too rusty :) |
| sha=sha, file_sha=file_sha, url=url)) | ||
| try: | ||
| write_cache(sha, file_contents) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This writes the tarball to a file named after its sha, not after the "normal" filename, which is not so great if one wants to manually download the file and insert it in the cache for example. Using the "normal" filename (something like jquery-foo.tar.gz) doesn't seem to be problematic, or is it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I think this is ok, it writes it to it's sha in the cache (and I don't think we want to document and support people writing thingsto the cache). The directions are about where to un-pack the tarballs into the source tree.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Moving the discussion to#13450.
@tacaswell we are not allowed to download anything during debian package build, so i'm gonna have to skip it and use the package jquery instead (JFYI) |
Note that you can just put a copy of the jquery tarball of the cache dir and matplotlib will find it there (@tacaswell perhaps this warrants a note in the docs?) |
we dont really have access to the jquery tarball; i'll figure out a way (i havent checked the actual code yet) |
Can't you just say that building matplotlib requires the matplotlib tarball and the jquery tarball? (don't know anything about the debian packaging system but on arch linux that would just mean putting both of them in the $sources array). |
the idea is that we prefer not to have multiple copies of the same software, in case a security fix is required (how can you find all the places where that software is?) |
ah, I guess you used to just patch the whole jquery away previously to point to your own version? then you can probably just do the same here? |
| except Exception as ex: | ||
| raise ex | ||
| else: | ||
| raise IOError("Failed to download FreeType. Please download " + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@tacaswell I don't think this clause can ever be triggered? In the for loop above, either you immediately break, or you immediately rethrow the exception, in fact I don't think you can even ever try the second url...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
yup, thatraise should be pass.
@sandrotosi it also depends where you pull our source from. If you pull the sdist from pypi then it will already have jquery vendored. |
You know, I forgot about this before, but there's also the XStatic family of packages we could use:XStatic-jQuery /XStatic-jQuery-UI. @sandrotosi I madethis patch for Fedora. |
that's a cute project... |
Regarding this I have an issue with downloading jquery-ui :( I do a regular build and get: error: Failed to download jquery-ui. Please download https://jqueryui.com/resources/download/jquery-ui-1.12.1.zip and extract it to build/bdist.linux-x86_64/egg/matplotlib/backends/web_backend/. Before the build I do: wget -o jquery-ui-1.12.1.zip<url>pushd lib/matplotlib/backends/web_backendunzip<path-to>/jquery-ui-1.12.1.zippopdpython setup.py build Which places the sources correctly! I then get: error: Failed to download jquery-ui. Please download https://jqueryui.com/resources/download/jquery-ui-1.12.1.zip and extract it to build/bdist.linux-x86_64/egg/matplotlib/backends/web_backend/. Note, this works for Py3 and matplotlib 3.0.3, but not for Py2 and matplotlib 2.2.4. Is there someway of specifying an external file position of the jquery-ui file? In general I think it would be ill-advised to require a web-connection for installation processes. |
@zerothi ,where are you getting the source from? If you pull the tarball from pypi it should have jquery pre-downloaded. We went back on forth on this a bit and settled on downloading jquery at installation / sdist creation time being the best option to avoid having to continue to vendor jquery-ui. Very odd that it works for py3 but not py2... |
I downloaded from github with Yes, I am also puzzled by this fact. I had the exact same procedure and also checked hash etc. |
@tacaswell I now downloaded: and used a custom # setup.cfg[directories]basedirlist = /opt/generic/gen-freetype/2.8.1[test]local_freetype = Falseand ran: python setup.py configpython setup.py buildpython setup.py install --prefix /path/to/install And I still fails at error: Failed to download jquery-ui. Please download https://jqueryui.com/resources/download/jquery-ui-1.12.1.zip and extract it to build/bdist.linux-x86_64/egg/matplotlib/backends/web_backend/. The Edit setup.cfg to change the build optionsBUILDING MATPLOTLIB matplotlib: yes [2.2.4] python: yes [2.7.16 (default, Mar 28 2019, 13:39:37) [GCC 8.3.0]] platform: yes [linux2]REQUIRED DEPENDENCIES AND EXTENSIONS numpy: yes [version 1.16.2] install_requires: yes [handled by setuptools] libagg: yes [pkg-config informationfor'libagg' could not be found. Usinglocal copy.] freetype: yes [version 2.8.1] png: yes [version 1.6.36] qhull: yes [pkg-config informationfor'libqhull' could not be found. Usinglocal copy.]OPTIONAL SUBPACKAGES sample_data: yes [installing] toolkits: yes [installing] tests: no [skipping due to configuration] toolkits_tests: no [skipping due to configuration]OPTIONAL BACKEND EXTENSIONS macosx: no [Mac OS-X only] qt5agg: yes [installing, Qt: 5.7.1, PyQt: 5.9; PySide2 not found] qt4agg: no [PySide not found; PyQt4 not found] gtk3agg: no [Requires pygobject to be installed.] gtk3cairo: no [Requires cairocffi or pycairo to be installed.] gtkagg: no [Requires pygtk] tkagg: yes [installing; run-time loading from Python Tcl / Tk] wxagg: no [requires wxPython] gtk: no [Requires pygtk] agg: yes [installing] cairo: no [cairocffi or pycairo not found] windowing: no [Microsoft Windows only]OPTIONAL LATEX DEPENDENCIES dvipng: yes [version 1.15] ghostscript: yes [version 9.26] latex: yes [version 3.14159265] pdftops: yes [version 0.48.0]OPTIONAL PACKAGE DATA dlls: no [skipping due to configuration]running config |
jdemeyer commentedJun 20, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
This doesn't actually seem to work:#14585 (to be precise: it does have jquery-ui in the tarball, but it downloads jquery-ui anyway) |
Uh oh!
There was an error while loading.Please reload this page.
attn@tacaswell, xref#11199
What works:
python setup.py sdistandpython setup.py bdist_wheelfetch jqueryand include it in the sdist/wheel.
pip install .fetches jquery and puts it to the correct location.pip install -e .fetches jquery and puts it in the source tree.Should be Py2-compatible, so backportable to 2.2.4.
(previous notes, now fixed)
PR Summary
PR Checklist