v8 5.8.48 #9343
v8 5.8.48 #9343
Conversation
|
@BrewTestBot test this please |
| cd output_path do | ||
| lib.install Dir["lib*"] | ||
| bin.install "d8" | ||
| end |
There was a problem hiding this comment.
👍
Could even do bin.install "d8" => "v8" IMHO. The v8 shell in the code base is just an example with little real-world use — d8 should be used instead. But it makes sense to give it a more appropriate name (just like "ch" => "chakra" when installing brew install chakra).
There was a problem hiding this comment.
@mathiasbynens can I amend this commit and force push to the branch? Or it is ok to have two commits? I see there is a rule of atomic commits: one commit - one formula, one formula - one commit. So want to make sure.
There was a problem hiding this comment.
Indeed. I've been going to make that (d8 => v8), but was not sure is it ok :)
There was a problem hiding this comment.
Since GitHub supports squash-and-merge nowadays, I’d assume Homebrew core devs are fine with either. Not sure though
There was a problem hiding this comment.
@mathiasbynens pushed as separated commit. I can change it if will be needed :) Thanks for suggestion!
|
5.8 breaks BC with 5.1 which is default now. Maybe let's use versioned formula - |
|
To clarify: v8 releases doesn't follow semantic versioning at all: it even breaks BC in path releases, which happens on everyday basis, with php-v8 I track v8 changes to public API almost on everyday basis. If there are packages who depends on default I'd suggest just bring my P.S.: Apart of that - 👍, thanks for bringing it up! |
|
Versioning is being discussed here: Homebrew/brew#620 |
|
@mathiasbynens I'm aware of, thanks. Do you think it would be better to move further discussion about this PR BC & versioning there? |
|
@pinepain Thanks for your work on creating and maintaining V8 formulae! My comment wasn’t aimed at you specifically — it was meant as a useful remark for anyone reading this thread now that you’ve brought up versioning.
@ilovezfs already said as much, so yeah :) FWIW your proposal sounds 👍 to me. |
|
For further readers, about versioning in homebrew - http://docs.brew.sh/Versions.html. @mathiasbynens got it, good remark, thank you. Last time @ilovezfs opened my eyes with poiting to that discussion which i somehow missed. I originally asked here as last time i brought the issue about v8 versioning it come up that no one was interested in that and that that time versions repository, which is deprecated now, had a strict rules about number of versions to support. Will bring it up there and create few PRs if we come up to some decision. As to exposing d8 as v8, I'd rather symlink or create some proxy script (see my note on diff). |
@oaleynik Does this formula builds all necessary headers and libs so that other devs can reuse it, or it d8 only, as you originally wrote? If it d8-only, it limit it usage significantly and definitely breaks BC, at this time at least |
|
@pinepain it is |
|
IMHO for me it feels that |
|
@oaleynik 👍, one day it would be this way, but nowadays it may or even will unnecessary break things we may not even aware of. I'd suggest to go with some FTR, latest stable is 5.6, not 5.8. |
|
The idea is to have the way to check latest features. Not use the latest stable version. But I agree that |
|
@oaleynik I would also to have latest feature =), but v8 releases pop up almost every hour, sometimes even more often. You may want to build them with So will you be OK if I just PR my formula to homebrew-core (sure, adjusting to 5.8)? Then you can go with simple |
|
@pinepain I agree with either. IMHO, before installing anything I would know what I need (dev deps or shell or whatever...) so I don't see an issue with having |
|
@oaleynik have you looked what formulae depends on P.S. having |
|
@pinepain in and one which has I'm not sure about all the taps around |
As long as we don't break these I'm fine with updating the V8 version here. |
| sha256 "487f2ca72096ee27d13533a6dad2d472a92ba40ef518a45226f19e94d4a79242" => :el_capitan | ||
| sha256 "dc9af3e08eda8a4acd1ff3c6b47a4c5170a92dbab7d2d79958a14d8aa42eefac" => :yosemite | ||
| sha256 "7bcd1bbd66c11305eeea0c36ca472de8a639f511abe0909c8815b1208dbce7b6" => :mavericks | ||
| end |
There was a problem hiding this comment.
Leave this as-is, it'll be updated when we merge.
|
@MikeMcQuaid this PR is fundamentally broken, not even mention that it breaks dependencies (not only listed one). So please, don't even consider to merge it. I would vote to use my PR #9392 which also provides working d8 (d8 from this PR won't work properly because of how ICU data file loaded by default, in referenced PR I patched that). |
@pinepain It may be but it can be improved. I'll comment on your PR. |
|
@MikeMcQuaid sure it could be, but even if we copy the formula from my PR we can't do such update in |
We should try to fix these deps and report issues to upstream projects. In this PRs case it only seems to break a single formula. |
|
@MikeMcQuaid this PR drop all .dylib building, so any deps won't work with it. As well as relocating d8 will lead to semi-working shell (see my comments above). Also, as you mentioned, 5.8 is not a stable (5.6 is). But @oaleynik interested in having latest v8 shell with latest ES features, if I get it right. While I'm absolutely for this changes, I'd rather keep them in a separate version, like I submitted.
|
That's fine. That's our typical process for major upgrades like this. |
We should continue to build dylibs or have dependencies build statically. |
|
OK, let's sum up what we want to achieve:
P.S.: 5.8.48 or 5.8.60 doesn't matter at all as v8 releases pop up few times a day. FTR: https://omahaproxy.appspot.com/ shows what v8 version considered as a "stable". Effectively stable for v8 means that it is inclued in stable chrome build and not that it stable by itself. |
|
If we upgrade v8 it should be to a stable version. We could consider adding 5.8 as a |
|
@MikeMcQuaid could |
|
@oaleynik ftr, I added v8@5.8 to pinepain/devtools, so if you up to give it a try, just |
It'll have to be maintained indefinitely and it's not a stable release which is against our packaging guidelines. |
oaleynik
force-pushed
the
v8__update-to-5-8-48
branch
from
47b0a90
to
06e47e5
Compare
oaleynik
force-pushed
the
v8__update-to-5-8-48
branch
from
06e47e5
to
8912a43
Compare
|
@MikeMcQuaid I've cleared all the audit errors. This is my first PR to Homebrew - I've missed initially that I can't run audit on my fork (or probably I've done it wrong). Branch has been rebased over latest upstream master and all intermediary commits have been squashed. Also, I've set version to 5.6.326 which is the version of V8 used in Chrome stable. It is still possible to build from HEAD. @pinepain there is no @pinepain Although I found your #9343 (comment) a bit insulting and disrespectful, which can turndown any desire to continue contributing to Homebrew - I highly appriciate all your help and suggestions. I believe that you have not had in mind any means to attack somebody or be aggressive. As I said a few comments above your concerns regarding such an optimistic update of v8 are reasonable. But I also think that having latest (well, at least stable, with an ability to build from HEAD) V8 under I'm not at the position of making the final decision here, however I believe that all together we can find the optimal solution! |
|
@oaleynik I'm sorry that you found it in this way, though it was targets not your personality, but the profound effect of what this changes will make on all dependencies. It you want to contribute, just take formula from my pr, rename formula class to
That's true, but when it appears, I don't have to add one. P.S.: I'm deeply sorry that you found it this way. |
@MikeMcQuaid yeah, makes sense, then I'll keep it in a separate tap as before. |
Just FYI: "I'm sorry that you X" is not an apology (it doesn't imply your actions are at fault but the other person was) and I agree that there's definitely more diplomatic ways of phrasing things than "fundamentally broken". Let's all try and assume the best in each other: everyone here is trying to make Homebrew better.
If you're not going to be making your own bottles then a |
|
To prevent further misunderstanding, I'll drop few lines in Russian. @oaleynik Олег, я действительно не хотел ничего такого обидного написать, извини. Основная идея в первоначальном виде данные изменения реально ломает очень много чего, т.е. не просто зависимости он ломает, а даже концепцию данной формулы. d8 просто консольная утилита, к слову, довольно примитивная если сравнивать с нодой (src), в то время как основная логика здесь в динамических библиотеках, сборка которых и выхлоп собственно очень сильно поменялась. Раньше собирались статические и динамические библиотеки, но примерно в 5.4 в v8 наконец-то довернули систему сборки и статические стали ненужны. И если для зависимостей решить проблему сборки путем исключения линкования статических библиотек не есть большая проблема, то вот тот ворох изменений в API v8 вряд ли кто-то будет разгребать. Вот собственно что я и имел в виду, но изложил из рук вон плохо. Впредь я таки лучше распишу лишний раз чем просто писать "fundamentally broken".
|
We cannot. That's another good case for a separate formula in your tap (until we need it in homebrew-core).
Yep.
If someone submits a PR for it: yep. There's no active plans for feature issue tracking this currently. |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
brew install --build-from-source <formula>, where<formula>is the name of the formula you're submitting?brew audit --strict <formula>(after doingbrew install <formula>)?This defaults v8 to 5.8.48, targets
d8only and allows build latest HEAD with --HEAD optionBased on: https://github.com/pinepain/homebrew-devtools/blob/master/Formula/v8%405.7.rb