Skip to content

Building extensions fail if not using latest patch #12571

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
Merged
ajbozarth merged 2 commits into from
May 12, 2022
Merged

Building extensions fail if not using latest patch #12571

ajbozarth merged 2 commits into from
May 12, 2022

Conversation

@ajbozarth
Copy link
Member

@ajbozarth ajbozarth commented May 12, 2022

Due to my changes in #12533
which were included in v3.4.1 extension builds may fail without an explicit
upgrade to the new patch (should not be necessary for patches).

This is because my change will compare against the exact version installed
rather than the range defined in the package.json, this update makes the
new check conditional to the original failure it was addressing.

References

Original source of the bug #12533

@ajbozarth
Building extensions fail if not using latest patch
42c9fc7 
Due to my changes in jupyterlab#12533
which were included in v3.4.1 extension builds may fail without an explicit
upgrade to the new patch (should not be necessary for patches).

This is because my change will compare against the exact version installed
rather than the range defined in the package.json, this update makes the
new check conditional to the original failure it was addressing.
@ajbozarth ajbozarth requested a review from fcollonval May 12, 2022 20:28
@ajbozarth ajbozarth self-assigned this May 12, 2022
@jupyterlab-probot
Copy link

jupyterlab-probot bot commented May 12, 2022

Thanks for making a pull request to jupyterlab!
To try out this branch on binder, follow this link: Binder

@ajbozarth
Copy link
Member Author

ajbozarth commented May 12, 2022

@fcollonval we should get this into a v3.4.2 asap to prevent any dev thrash. Currently if you have Jupyterlab 3.4.0 pip installed and then build your extension the extension will pull in 3.4.1 dependencies during the build and @juyterlab/builder will error since 3.4.0 is installed even though that difference should be fine because it checks against 3.4.0 specifically instead of ^3.4.0 or ~3.4.0 that are usually set in the dependencies.

@jtpio
Copy link
Member

jtpio commented May 12, 2022

Thanks for the quick fix!

Also noticed this in voila-dashboards/voila#1155 and jupyterlite/xeus-python-kernel#5 today.

Backporting to 3.4.x and making a new 3.4.2 release right after sounds good 👍

@jtpio jtpio added the bug label May 12, 2022
@jtpio jtpio added this to the 3.4.x milestone May 12, 2022
@jtpio
Copy link
Member

jtpio commented May 12, 2022

@ajbozarth do you feel like making the 3.4.2 release with the releaser? Otherwise no problem it could be another time.

@jtpio jtpio mentioned this pull request May 12, 2022
Closed
@ajbozarth
Copy link
Member Author

ajbozarth commented May 12, 2022

Yeah I can do the 3.4.2 release, it'll be my first time doing it myself (rather than watching someone doing it on a call), but iirc it's pretty straightforward and I should have all the correct perms.

If you approve this I can merge it once the CI finishes then backport it and merge that after it's CI passes then I'll start the release. (I should have enough time left in my day to do all that)

@jtpio
Copy link
Member

jtpio commented May 12, 2022

Sounds good.

For reference the diff in https://github.com/jupyterlab/jupyterlab/pull/12533/files shows the following CodeQL warning, which could have given a hint (not sure the warning was there at the time of the PR though):

image

jtpio
jtpio approved these changes May 12, 2022
View reviewed changes
Copy link
Member

@jtpio jtpio left a comment

Choose a reason for hiding this comment

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

Thanks!

@ajbozarth
Copy link
Member Author

ajbozarth commented May 12, 2022

yeah I saw that warning too, it seems to have shown up after we merged the pr

@ajbozarth
Copy link
Member Author

ajbozarth commented May 12, 2022

The test failures are unrelated, merging

@ajbozarth ajbozarth merged commit 418066f into jupyterlab:master May 12, 2022
@ajbozarth ajbozarth deleted the builder_bug branch May 12, 2022 21:37
@ajbozarth
Copy link
Member Author

ajbozarth commented May 12, 2022

@meeseeksdev please backport to 3.4.x

meeseeksmachine pushed a commit to meeseeksmachine/jupyterlab that referenced this pull request May 12, 2022
@ajbozarth @meeseeksmachine
Backport PR jupyterlab#12571: Building extensions fail if not using l…
17713a6 
…atest patch
@meeseeksmachine meeseeksmachine mentioned this pull request May 12, 2022
Merged
ajbozarth added a commit that referenced this pull request May 12, 2022
@ajbozarth
Merge pull request #12572 from meeseeksmachine/auto-backport-of-pr-12…
1c8f008 
…571-on-3.4.x

Backport PR #12571 on branch 3.4.x (Building extensions fail if not using latest patch)
@github-actions
Copy link
Contributor

github-actions bot commented May 12, 2022

Benchmark report

The execution time (in milliseconds) are grouped by test file, test type and browser.
For each case, the following values are computed: min <- [1st quartile - median - 3rd quartile] -> max.

The mean relative comparison is computed with 95% confidence.

Results table
Test file large_code_notebook large_md_notebook
open
chromium
actual 17857 <- [18122 - 18275 - 18438] -> 21097 4334 <- [4561 - 4622 - 4681] -> 4854
expected 17516 <- [17916 - 18152 - 18538] -> 19794 4370 <- [4523 - 4571 - 4639] -> 4942
Mean relative change 0.7% ± 0.7% 0.8% ± 0.6%
switch-from
chromium
actual 713 <- [850 - 868 - 885] -> 951 632 <- [689 - 698 - 710] -> 780
expected 735 <- [851 - 867 - 885] -> 930 658 <- [685 - 697 - 710] -> 765
Mean relative change 0.3% ± 0.9% 0.3% ± 0.8%
switch-to
chromium
actual 455 <- [565 - 623 - 632] -> 675 1116 <- [1150 - 1165 - 1187] -> 1250
expected 456 <- [556 - 617 - 629] -> 681 362 <- [1144 - 1159 - 1184] -> 1244
Mean relative change 1.5% ± 2.0% 1.0% ± 1.5%
close
chromium
actual 796 <- [1464 - 1488 - 1515] -> 1597 774 <- [803 - 820 - 834] -> 901
expected 796 <- [1458 - 1481 - 1506] -> 1651 766 <- [808 - 821 - 834] -> 925
Mean relative change -0.2% ± 2.0% -0.3% ± 0.8%

Changes are computed with expected as reference.

@martinRenou martinRenou mentioned this pull request May 13, 2022
Closed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jtpio jtpio jtpio approved these changes

@fcollonval fcollonval Awaiting requested review from fcollonval

Assignees

@ajbozarth ajbozarth

Labels

bug

Projects

None yet

Milestone

3.4.x

Development

Successfully merging this pull request may close these issues.

2 participants

@ajbozarth @jtpio