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.

Sign up for GitHub

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

Jump to bottom

py-jupyterlab-pygments: install from wheel to avoid cyclic dependency #33278

Merged
merged 9 commits into from
Oct 17, 2022
Merged

py-jupyterlab-pygments: install from wheel to avoid cyclic dependency #33278

merged 9 commits into from
Oct 17, 2022

Conversation

iarspider
Copy link
Contributor

See also: jupyterlab/jupyterlab_pygments#23 . Alternatively, we can install py-jupyterlab-pygments from wheel, since py-jupyterlab dependency is build-time only.

@adamjstewart
Copy link
Member

Good catch, and thanks for the issue link!

Can you add the issue link as a comment in the package? That will help us track whether or not the cycle is ever removed.

Does this build correctly? I'm a bit concerned about what will happen if jupyterlab isn't installed. It may be better to install from a wheel like you suggested. Then we can remove all build-only deps.

I'm told that there is work in progress to change Spack's concretizer such that the same dependency can appear in the DAG multiple times. This is important for cross-compilation, but may also help here. Since we're dealing with build-only dependencies, it should be possible to turn this cycle into a loop-the-loop, where different versions of build deps are used than run deps.

@iarspider iarspider changed the title py-jupyterlab-pygments: avoid cyclic dependency py-jupyterlab-pygments: install from wheels avoid cyclic dependency Oct 13, 2022
@iarspider iarspider changed the title py-jupyterlab-pygments: install from wheels avoid cyclic dependency py-jupyterlab-pygments: install from wheel to avoid cyclic dependency Oct 13, 2022
@adamjstewart
Copy link
Member

Can you keep the comment and link about cyclic deps so we know why we're using a wheel?

@adamjstewart adamjstewart self-assigned this Oct 13, 2022
adamjstewart
adamjstewart previously approved these changes Oct 13, 2022
View reviewed changes
@iarspider
Copy link
Contributor Author

@spackbot fix style

@spackbot-app
Copy link

spackbot-app bot commented Oct 13, 2022

Let me see if I can fix that for you!

@spackbot-app
Copy link

spackbot-app bot commented Oct 13, 2022

I was able to run spack style --fix for you!

spack style --fix 
==> Running style checks on spack
  selected: isort, mypy, black, flake8
==> Modified files
  var/spack/repos/builtin/packages/py-jupyterlab-pygments/package.py
==> Running isort checks
  isort checks were clean
==> Running mypy checks
Success: no issues found in 556 source files
  mypy checks were clean
==> Running black checks
reformatted var/spack/repos/builtin/packages/py-jupyterlab-pygments/package.py
All done! ✨ 🍰 ✨
1 file reformatted.
  black checks were clean
==> Running flake8 checks
var/spack/repos/builtin/packages/py-jupyterlab-pygments/package.py:15: [E501] line too long (110 > 99 characters)
  flake8 found errors
Keep in mind that I cannot fix your flake8 or mypy errors, so if you have any you'll need to fix them and update the pull request. If I was able to push to your branch, if you make further changes you will need to pull from your updated branch before pushing again.

I've updated the branch with isort fixes.

@iarspider iarspider closed this Oct 13, 2022
@iarspider iarspider reopened this Oct 13, 2022
@adamjstewart adamjstewart enabled auto-merge (squash) October 14, 2022 14:27
@iarspider
Copy link
Contributor Author

@spackbot run pipeline

@spackbot-app
Copy link

spackbot-app bot commented Oct 17, 2022

I've started that pipeline for you!

@adamjstewart adamjstewart merged commit f3523d8 into spack:develop Oct 17, 2022
@iarspider iarspider deleted the patch-23 branch October 17, 2022 13:47
luke-dt added a commit to dantaslab/spack that referenced this pull request Oct 17, 2022
@luke-dt @haampie
@jedwards4b @iarspider @scottwittenburg @adamjstewart @bonachea @aurianer @eugeneswalker @balay @jlnav @eschnett @gsjaardema @michaelkuhn @blue42u @mkstoyanov @fangohr @giordano @dialvarezs @wdconinc
merge with upstream (#7)
e552c78 
* py-execnet: 1.9.0 (spack#33282)

* py-execnet: 1.9.0

* bounds

* add version 1.12.3 of parallel-netcdf (spack#33286)

* Add checksum for py-psutil 5.9.2 (spack#33139)

* gitlab ci: Print better information about broken specs (spack#33124)

When a pipeline generation job is automatically failed because it
generated jobs for specs known to be broken on develop, print better
information about the broken specs that were encountered.  Include
at a minimum the hash and the url of the job whose failure caused it
to be put on the broken specs list in the first place.

* meson: remove slash in path (spack#33292)

* Add checksum for py-gitpython 3.1.27 (spack#33285)

* Add checksum for py-gitpython 3.1.27

* Update package.py

* Update var/spack/repos/builtin/packages/py-gitpython/package.py

Co-authored-by: Adam J. Stewart <ajstewart426@gmail.com>

Co-authored-by: Adam J. Stewart <ajstewart426@gmail.com>

* UPC++/GASNet-EX 2022.9.0 update (spack#33277)

* gasnet: Add new release hash
* upcxx: Add new release hash
* gasnet: misc updates
* upcxx: misc updates

* Fix pika@0.9.0 sha (spack#33307)

* hip@5.2.0 onwards: set prefix properly (spack#33257)

* hip-set-prefix-rocm5.2.0-onwards

* Update var/spack/repos/builtin/packages/hip/package.py

Update description

Co-authored-by: Satish Balay <balay@mcs.anl.gov>

* petsc,py-petsc4py,slepc,py-slepc4py: add version 3.18.0 (spack#32938)

* petsc,py-petsc4py,slepc,py-slepc4py: add version 3.18.0

* workaround for dealii build failure [with petsc version check]

* pism: add compatibility fix to for petsc@3.18

* add in hipsolver dependency

* py-libensemble: updating package for v0.9.3 (spack#33298)

* commit updating py-libensemble package for 0.9.3

* removed commented-out lines

* Add checksum for py-oauthlib 3.2.1 (spack#33201)

* Add checksum for py-oauthlib 3.2.1

* Update package.py

* [@spackbot] updating style on behalf of iarspider

* Update package.py

* Update package.py

Co-authored-by: iarspider <iarspider@users.noreply.github.com>

* ninja: New version 1.11.1 (spack#33215)

* seacas: update to latest release (spack#33330)

Add checksum for latest tag/release

* gptl: new version 8.1.1; use the correct mpi fortran compiler (spack#33235)

* gptl: new version 8.1.1; use the correct `mpifc`
* add `F90` and `$F77`

* glib: add 2.74.0 and 2.72.4 (spack#33332)

* depfile: update docs (spack#33279)

* rocksdb: add 7.7.3 (spack#33341)

* Add checksum for py-seaborn 0.12.0 (spack#33145)

* Add checksum for py-seaborn 0.12.0

* Update package.py

* Update package.py

* [@spackbot] updating style on behalf of iarspider

* Update package.py

Co-authored-by: iarspider <iarspider@users.noreply.github.com>

* CI: allow multiple matches to combine tags (spack#32290)

Currently "spack ci generate" chooses the first matching entry in
gitlab-ci:mappings to fill attributes for a generated build-job,
requiring that the entire configuration matrix is listed out
explicitly. This unfortunately causes significant problems in
environments with large configuration spaces, for example the
environment in spack#31598 (spack.yaml) supports 5 operating systems,
3 architectures and 130 packages with explicit size requirements,
resulting in 1300 lines of configuration YAML.

This patch adds a configuraiton option to the gitlab-ci schema called
"match_behavior"; when it is set to "merge", all matching entries
are applied in order to the final build-job, allowing a few entries
to cover an entire matrix of configurations.

The default for "match_behavior" is "first", which behaves as before
this commit (only the runner attributes of the first match are used).

In addition, match entries may now include a "remove-attributes"
configuration, which allows matches to remove tags that have been
aggregated by prior matches. This only makes sense to use with
"match_behavior:merge". You can combine "runner-attributes" with
"remove-attributes" to effectively override prior tags.

* meson: update OneAPI compiler support patch (spack#33293)

* py-tensorflow: fix zlib (spack#33349)

* py-tensorflow: fix zlib

* [@spackbot] updating style on behalf of haampie

Co-authored-by: haampie <haampie@users.noreply.github.com>

* py-meson-python: add new versions (spack#33294)

* tasmanian: disable openmp by default (spack#33345)

* octopus: upgrade to 12.1 (spack#33343)

* py-sphinx: add v5.3 and v5.2 (spack#33356)

* py-setuptools: add v65.5.0 (spack#33353)

* libblastrampoline: Add versions 5.1.1, 5.2.0 (spack#33352)

* nextflow: add v20.10.0 (spack#33354)

* sdl2: add v2.0.22 and v2.24.1 (spack#33351)

* mariadb-c-client: add 3.3.2, 3.2.7, 3.1.18, 3.0.10 (spack#33335)

* py-tensorflow-hub: zlib, again. (spack#33359)

* Add checksum for py-sniffio 1.3.0 (spack#32975)

* py-numpy: add v1.23.4 (spack#33260)

* py-jupyterlab-pygments: install from wheel to avoid cyclic dependency (spack#33278)

* py-jupyterlab-pygments: avoid cyclic dependency

* Fix style

* Update package.py

* Update package.py

* [@spackbot] updating style on behalf of iarspider

* Update package.py

* Flake-8

* fix

Co-authored-by: iarspider <iarspider@users.noreply.github.com>

* installer.py: show timers for binary install (spack#33305)

Print a message of the form
```
Fetch mm:ss.  Build: mm:ss.  Total: mm:ss
```
when installing from buildcache. 

Previously this only happened for source builds.

* Add checksum for py-astroid 2.12.7, py-astroid 2.12.10, py-setuptools 62.6.0, py-wrapt 1.14.1, py-pylint 2.15.0 (spack#32976)

* Add checksum for py-astroid 2.12.7, py-setuptools 62.6.0

* Also add checksum for py-wrapt

* Update package.py

* Update package.py (spack#57)

* Update package.py

* Update package.py

* Update package.py

* gitlab ci: Do not force protected build jobs to run on aws runners (spack#33314)

* installer.py: fix/test get_deptypes (spack#33363)

Fixing an oversight in spack#32537

`get_deptypes` should depend on new `package/dependencies_cache_only`
props.

* Add checksum for py-grpcio-tools 1.48.1 (spack#33358)

* Add checksum for py-prompt-toolkit 3.0.31 (spack#33362)

Co-authored-by: Harmen Stoppels <harmenstoppels@gmail.com>
Co-authored-by: Jim Edwards <jedwards@ucar.edu>
Co-authored-by: iarspider <iarspider@gmail.com>
Co-authored-by: Scott Wittenburg <scott.wittenburg@kitware.com>
Co-authored-by: Adam J. Stewart <ajstewart426@gmail.com>
Co-authored-by: Dan Bonachea <dobonachea@lbl.gov>
Co-authored-by: Auriane R <48684432+aurianer@users.noreply.github.com>
Co-authored-by: eugeneswalker <38933153+eugeneswalker@users.noreply.github.com>
Co-authored-by: Satish Balay <balay@mcs.anl.gov>
Co-authored-by: John-Luke Navarro <33131245+jlnav@users.noreply.github.com>
Co-authored-by: iarspider <iarspider@users.noreply.github.com>
Co-authored-by: Erik Schnetter <schnetter@gmail.com>
Co-authored-by: Greg Sjaardema <gsjaardema@gmail.com>
Co-authored-by: WuK <i@wu-kan.cn>
Co-authored-by: Michael Kuhn <michael.kuhn@ovgu.de>
Co-authored-by: Jonathon Anderson <17242663+blue42u@users.noreply.github.com>
Co-authored-by: haampie <haampie@users.noreply.github.com>
Co-authored-by: Miroslav Stoyanov <30537612+mkstoyanov@users.noreply.github.com>
Co-authored-by: Hans Fangohr <fangohr@users.noreply.github.com>
Co-authored-by: Mosè Giordano <giordano@users.noreply.github.com>
Co-authored-by: Diego Alvarez <dialvarezs@gmail.com>
Co-authored-by: Wouter Deconinck <wdconinc@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adamjstewart adamjstewart adamjstewart approved these changes

@pradyunsg pradyunsg Awaiting requested review from pradyunsg

Assignees

@adamjstewart adamjstewart

Labels
new-version python update-package
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@iarspider @adamjstewart