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

webkitgtk222x: init at 2.22.2 #47949

Merged
merged 2 commits into from Oct 9, 2018
Merged

webkitgtk222x: init at 2.22.2 #47949

merged 2 commits into from Oct 9, 2018

Conversation

yrashk
Copy link
Contributor

@yrashk yrashk commented Oct 6, 2018

Motivation for this change

Some applications require webkitgtk 2.22.x. For example, when trying to upgrade astroid to 0.14, I found about this requirement astroidmail/astroid#567

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Fits CONTRIBUTING.md.

@yrashk
Copy link
Contributor Author

yrashk commented Oct 6, 2018

I am not sure what's the best way to add 2.22.x but this is my first attempt at it without disrupting anything else. Please let me know what kind of adjustments are necessary to complete this. I'd like to get this merged in order to upgrade astroid to 0.14

@andir
Copy link
Member

andir commented Oct 6, 2018

Thank you for working on this! It does look okay but besides the version and the hashsum is just a copy of the older branch.

Are you able to verify which of the depending package we be moved to the newer 2.22.x branch?

Depending on that outcome I'd just bump the other expression (or remove it) OR propose writing a generic build expression that we can feed version & hash like we do at many other places.

Last month there was a new security release of the library (https://webkitgtk.org/security/WSA-2018-0007.html) listing a few issues that are only fixed in the 2.22.x branch. It would be nice to have those addressed / fixed :-)

@yrashk
Copy link
Contributor Author

yrashk commented Oct 6, 2018

@andir yes, it is a verbatim copy with the version & hash changed. Just wanted to get the ball rolling.

It appears that there's a good number of packages depending on webkitgtk. What would be the best way to ensure their transition to 2.22.x?

Also, can we take a gradual approach? Add webkitgtk222x first so it is available for things like astroid and then move others to it and once the transition is done, alias webkitgtk222x as webkitgtk?

Any advice is highly appreciated.

@andir
Copy link
Member

andir commented Oct 6, 2018

I think the gradual approach sounds reasonable. I (personally) would prefer if we would have at least a few (one?) consumers of the newer version when this PR is being merged.

@yrashk
Copy link
Contributor Author

yrashk commented Oct 6, 2018

I am ready to send a PR with astroid 0.14 (it doesn't work with 2.20.* as per the issue I referenced above) the moment this is merged.

@yrashk
Copy link
Contributor Author

yrashk commented Oct 6, 2018

Or I can include that astroid upgrade commit into this PR, if this works better.

@jtojnar
Copy link
Contributor

jtojnar commented Oct 6, 2018

I upgraded webkitgtk in #45950, there do not seem to be any issues.

@flokli
Copy link
Contributor

flokli commented Oct 6, 2018

@yrashk I'd say it's fine if we add webkitgtk222x and point astroid 0.14 to it in a second commit.
It's then not introduced without any consumer.
#45950 can then make 2.22.x a default and remove 2.20.5, as it currently also bumps and defaults 2.20.x to 2.22.x.

@andir
Copy link
Member

andir commented Oct 6, 2018

@jtojnar Would you mind if we go forward with this and you rebase your branch onto it later on? Having it in the tree already might allow things like the mentioned astroid upgrade to go in.

Also we can start merging some of your changes to move packages onto the new webkitgtk?

If it is just a bit longer for the gnome 3.30 PR to land that is fine with me and we just wait a few more days.. It has been (insecure & broken) for a while now..

@jtojnar
Copy link
Contributor

jtojnar commented Oct 6, 2018

I am going to merge the GNOME PR later today.

@yrashk
Copy link
Contributor Author

yrashk commented Oct 6, 2018

@andir just in case, I added the astroid commit to this PR, but otherwise waiting for the GNOME PR to land to reapply astroid upgrade on top of it.

@yrashk
Copy link
Contributor Author

yrashk commented Oct 6, 2018

Just to note, that PR refers to 2.22.0 at the moment (not 2.22.2 as mine)

@jtojnar
Copy link
Contributor

jtojnar commented Oct 7, 2018

I am still stuck with the meson review, I guess we can merge this for now.

@yrashk
Copy link
Contributor Author

yrashk commented Oct 9, 2018

@andir should this be merged, then? (since the GNOME PR is still not done)

@andir
Copy link
Member

andir commented Oct 9, 2018

@yrashk yes I think so. I am looking at it right now.

@andir andir merged commit a00b820 into NixOS:master Oct 9, 2018
@andir
Copy link
Member

andir commented Oct 9, 2018

@yrashk thank you for the contribution.

I'll see how much if I can get some time for checking what packages can already be updated (also on 18.03 & 18.09) without breaking.

@jtojnar jtojnar mentioned this pull request Nov 30, 2018
10 tasks
@hedning hedning mentioned this pull request Sep 12, 2019
20 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants