« first day (4279 days earlier)      last day (894 days later) » 

00:11
@ramsey OK. You;r eon your own then :-)
 
8 hours later…
08:10
hmmm... NEWS file in PHP-8.1 seems broken... (8.2.0alpha entries)
looks like the whole branch is broken (This branch is 1 commit ahead, 7 commits behind master. )
we need a git expert ;)
Looks like a clean fast forward to master to me.
Also PHP-8.0 does not appear to be fully merged to me. It contains one commit at the end that's in no other branch.
Based on CI it's very likely that 110573726b6917580cd2c0c5d89c83e72f2de686 is the "intended" state of PHP-8.1: github.com/php/php-src/actions?query=branch%3APHP-8.1
 
1 hour later…
cmb
cmb
09:26
I think the last good commit to PHP-8.1 was 26d63c74be46e3073254677545aa3f7350af8f62. Directly afterwards, somehow master got merged into PHP-8.1 or PHP-8.1 was rebased onto master.
We need to fix that.
2022-07-01 06:31 +0100 David CARLIER o │ Revert "Merge branch 'PHP-8.1'"
That... looks suspicious
I can try to rebuild this, but it's going to take time and force pushing
cmb
cmb
I think 6876c202ea08cac1a05a0461a677e45ea669692e was somehow the issue.
and that's going to make everybody's life harder
yeah, you can't revert merging branches like this
But we also should not wait fixing it
@cmb Based on gitk --all the one I mentioned ("Disallow assigning reference to unset readonly property") also LGTM.
Both PHP-8.1 and master need to be reset to the last good commit, and then the new commits need to be played back on it.
cmb
cmb
09:34
^ that
8.0 looks OK
cmb
cmb
right
Are you going to fix this?
Yes, can you email the list that everybody is going to have to recheckout the tree though? It's gonna be a pain. Also tell David that he should not do it like this ever again :-)
@cmb What do you reckon the last good one in master is?
I really don't believe that the revert is the issue. Reverting a merge is generally fine, you just can't merge the already merged commits again.
From what I see Girgias accidentally pushed master to 8.1 for the LMDB fixes.
cmb
cmb
He somehow merged master into PHP-8.1 prior to the attempted revert.
That's also possible. I think only David and George Peter had commits lately.
09:40
not in master
2022-07-04 09:40 +0000 Unknown o Untracked changes
2022-07-03 07:16 +0200 Sebastian Bergmann o [master] {origin/master} {origin/HEAD} [ci skip] Fix typo
2022-07-02 17:50 +0100 David Carlier o Fix GH-8907: Document forgotten API changes.
2022-07-02 18:10 +0100 Jakub Zelenka o Fix labeler selection of SAPIs
2022-07-01 16:59 +0100 George Peter Banyard o Pre-compute remote address length in CLI SAPI
it's hard to see what the last known good commit was
The LMDB one has the same date and message. So that looks like a broken rebase / cherry-pick.
Okay, so @Girgias messed up ;-)
The last merge by Ilija looks like I expect it to look.
github.com/php/php-src/actions?query=branch%3APHP-8.1 - The last commit before the broken one is the "Disallow assigning reference to unset readonly property" by Ilija.
cmb
cmb
I cannot pull from master beyond 1d0c287b9083769bd9f1f798f42ba5fd07257669
So, what changes do I need to make you reckon?
09:43
I would say that:

git checkout PHP-8.1
git reset --hard 110573726b6917580cd2c0c5d89c83e72f2de686
git push origin PHP-8.1:PHP-8.1 --force-with-lease
Then the branches should look like expected. Afterwards PHP-8.0 needs to be merged into 8.1 and 8.1 into master.
You can verify with gitk --all. My screenshot above is generated using that.
For the LMDB one you need to verify whether it is good in 8.0 or not, because that one is the one in question.
PHP-8.1: reset to 110573726b6917580cd2c0c5d89c83e72f2de686
add
ac6edbb73e701f6132b56065ff9343869feb4461

master: reset to 40908b10fc6a73f25606119a8d38ac19c8b39bb6
add
1d0c287b9083769bd9f1f798f42ba5fd07257669
55908db007816ecbb45fabae463f617bef4329cc
b37245b8da88c91d2799e1077a89bb37e069f95e
1c753a958bca9b3c802954bbb2d0681235e4af93
583cc01e9eaabeed77d5c8959ceb06e32792026d
fdc09e302a84cc05fd89bc906619f5a1a882178d
eb5b12c2255238f99adf1be06c46b7a1276311e4

merge PHP-8.1 into master
does that look right?
What does "--force-with-lease" do?
8.0 looks fine to me
No reset should be required for master. The cherry-pick of ac6edbb73e701f6132b56065ff9343869feb4461 in PHP 8.1 should not be necessary, because that one will be merged from 8.0.
--force-with-lease is "force, but verify that the local copy of the remote branch is up to date".
That is a safety check that verifies that you do not push over commits your local repo has never seen.
the cherry picks into master do need to happen though, right?
remote: error: GH006: Protected branch update failed for refs/heads/PHP-8.1.
remote: error: Cannot force-push to this protected branch
To github.com:php/php-src.git
! [remote rejected] PHP-8.1 -> PHP-8.1 (protected branch hook declined)
error: failed to push some refs to 'github.com:php/php-src.git'
No. Reset 8.1 to 110573, then force push. Afterwards merge 8.0 into 8.1, then 8.1 into master.
that again... I am an admin, dammit
you say no force push of master is needed either?
09:48
After the hard reset of 8.1 to 110573 it should look like this.
8.1 is then fully merged into master. This is the expected state. However 8.0 is not fully merged into 8.1.
@Derick I literally do not understand what I did wrong
@Girgias let's fix it first
@TimWolla Ah, OK
i first need to disable branch protection (Scary)
@Derick Yeah thanks for working on it, @IluTov send me a ping on discord and I've crawled out of bed to check my local branches
No force push of master is required. You'll just have the LMDB commit duplicated, but force pushing master is worse than the duplicated commit.
yeahj
@TimWolla OK, all done - want to verify?
09:51
Okay, let me check.
@Derick LGTM in a completely fresh clone.
cool, looks OK here too
@TimWolla What do you think went wrong?
Here's the screenshot. The marked commit was the last good commit in 8.1.
yeah
@Derick I suspect that Girgias believed he had checked out his local master, then wanted to merge the remote master using git merge origin/master. But he accidentally had the local PHP-8.1 checked out.
Because master is a strict superset of 8.1 this is a fast forward, which does not generate a new merge commit.
So the merge goes through completely cleanly and the push does as well.
@Girgias Does your shell tell you which branch you are on?
09:57
@TimWolla No, what seems to have happened is that I did a [girgias@localhost PHP-8.1]$ git rebase -i php/master when amending the NEWS file, that's the only explanation looking at my command history (which is a bit broken due to multiple terminal prompts)
@Derick I use worktrees
I don't know what "worktrees" are
So yes I have a big fat 8.0 and 8.1 in front
Worktrees are multiple checkouts of different branches for the same .git-blobstore.
man git worktree.
sounds dangerous :-D
cmb
cmb
nope, works fine
09:58
@Derick It's literally in the PHP Wiki wiki.php.net/vcs/gitfaq
It's pretty safe. You have a dedicated directory per branch, but without having multiple clones.
cmb
cmb
@Girgias why rebase? You can do git commit -a instead.
And git is aware of each of the worktrees and their state.
@cmb Because I'm an idiot?
cmb
cmb
:D
09:59
everybody star that comment ;-)
I think instead of amending I did the commit and I was like ... let me rebase this quickly
@Girgias That's effectively equivalent to the merge I described. Details are in git reflog.
A rebase without any additional commits is equivalent to a fast-forward.
right
I don't know how to git
@Derick Now that the issue is resolved you can update the list. The other users will only need to hard reset their 8.1.
git fetch
git checkout PHP-8.1
# Backup local changes
git reset --hard origin/PHP-8.1
# Reapply local changes
I'll email, cheers
10:02
And before first pushing ideally verify carefully that the graph looks good (e.g. using gitk).
cmb
cmb
^ that
i never really liked gitk, and either used gitg or tig... but I think only gitk showed the real issue
gitk is the only piece of git gui I am comfortable with, I use it just for visualization.
oh yeah, it's for visualisation only - i don't use a gui for committing
cmb
cmb
I think we need to revert github.com/php/php-src/commit/… in master.
10:06
@cmb That I can't tell. But the commit is not broken per se.
i.e. it shouldn't break the repository.
cmb
cmb
It's about the change; that should also target master (I would have targeted master only, but okay). The revert was likely done due to the merge confusion.
That's the "I can't tell part" :-)
OK, have to do some other things now. Back later.
cmb
cmb
Sure. And thanks for fixing this!
10:53
@Girgias TIL. That sounds useful.
cmb
cmb
11:03
I hate Alpine: github.com/php/php-src/issues/8917 (3 reports during 5 weeks)
11:41
@IluTov It is :D prevent the whole buildconf/configure just to changes and everything
11:54
@Girgias So that creates different directories? Then I'll have to switch IDE windows... I already foresee me messing that up.
@IluTov Yes
Well that is indeed an issue that could happen
Would've been nicer if the main directory was automatically symlinked or something.
12:13
Good Morning
 
1 hour later…
13:30
@Derick thanks for fixing the git repo
@RemiCollet I second that! Thanks, @Derick! That doesn't sound like it was very fun.
cmb
cmb
force-pushing is always fun :p
13:52
I was thinking about the strelen tests that are failing because of whitespace. As it's only diagnostic, would it be acceptable to mod the test to encode the output (JSON encode maybe?) to make the rest more resilient.
cmb
cmb
@MarkR Whitespace as in LF vs. CRLF?
and some trailing whitespace.
cmb
cmb
Also, why are you marked as first-time-contributor on github.com/php/php-src/pull/8912?
LF vs. CRLF shouldn't be a real issue. Don't convert to CRLF on Windows.
Trailing whitespace may not be nice, but I prefer to see strings in plain text, if possible.
well it's more that by default you can't see them at all. I was thinking representing them as \n \r etc might make it easier to spot
Also, I'm not sure about the contributor bit. Fair few listed on github.com/php/php-src/…
cmb
cmb
14:04
@MarkR ugh, it github.com/marandall your account?
It's an old one, I have marandall and markrandall
Where's that one being linked?
Humm, that's odd, they didn't come up when I searched for closed pr's with author:marandall
cmb
cmb
Interestingly, the PRs where created by markrandall, but the commits are attributed to marandall. That explains why GH marks markrandall as first time contributor. That is not an issue per se, but someone needs to trigger CI manually for all PR commits.
I'm guessing the easiest way will just be finishing up this warning enhancement PR then it'll register me as a contributor?
This will be the first one I've made since moving to GH fully and getting rid of the Karma system
14:10
Yes. Or you need to claim your email address with your new account.
@cmb At least the "Alpine" is written in the issue. It could be worse.
Could have been slackware.
cmb
cmb
Yeah, but 2 bug reports regarding the same issue within a few hours are not nice. And I expect more to come.
TIL that Slackware is alive. :|
"alive" might be a stretch.
15:21
Hi
15:40
Apparently an SSL security release is being made tomorrow (tuesday): twitter.com/jschauma/status/1543769368676417537
@TimWolla taps the sign
@Danack Personally I believe that one should default to Debian and ignoring Alpine is fine.
cmb
cmb
@Danack Yeah, they always release on our tagging days, what is annoying wrt. the Windows builds. However, doesn't look like a real issue in this case, since 1.1.1q only fixes moderate issues.
Disk space is cheap-ish and Alpine/musl is just too limited. I believe it still doesn't correctly support IPv6 in its DNS resolver.
Also the benefit of Alpine vs Debian goes away ones you use more than one container, because the lower layers will be shared across images. So you pay the Debian cost only once.
cmb
cmb
15:56
Tell this hub.docker.com/_/php; I think that's the main reason for the Alpine usage wrt. PHP.
Yes. Source repository for that is here: github.com/docker-library/php/issues. They are super responsive, so if there's anything they can improve (short of "discontinue alpine" maybe), I recommend to message them / create an issue.
The Docker Official Images project strives to follow upstream's recommendations.
Personally I maintain two of the official images (spiped, adminer) and help out with PRs for others. So I interact somewhat frequently with the maintainers.
You could also pop into #docker-library on libera.chat for lower barrier communication with them.
16:10
The alpine version is rather popular in my experience due to the smaller images
@MarkR that advantage mostly goes away once you add 150MB of ImageMagick or whatever it is.
It's changed a bit now but it used to be something like a 10x size difference when I was first using it.
and that was a lot without a CI pipeline that could build it for you
yeah......my recollection is that it prompted debian-slim to be a thing, which made the difference not be as much. And probably still makes sense if you're deploying thousands of containers, and less sense if you're deploying tens of containers.
If you're deploying thousands, then you hopefully have a local mirror instead of grabbing them from Docker Hub.
16:26
I usually run somewhere between a couple of dozen normally and a couple of thousand at peak, when I profiled it many many years ago the alpine ones loaded a fair bit faster, but that could have changed now
I remember the days where you could fit a whole Linux system (as a firewall) on one 1.44" disk
What, no punchcards?
17:28
@Girgias Do you have a practical use example of DNF types somewhere?
cmb
cmb
array|(Traversable&Countable) (see phpinternals.news/103, 5:41)
lol
:D
.....you did not just link him to his own podcast.
18:03
Hey. I'm working on the Curl URL RFC, but there are people that want an API close to the curl implementation, people who want a more hight level API, and some also want the CurlUrl object to be immutable. There is no clear consensus. If we want to get that into 8.2, we need to open the vote ASAP or it will be delayed until 8.3. Don't like to define this in a rush, but what do you think if we open a STV with 4 different choices, do nothing, and 3 different implementations ?
@cmb I'm personnaly not against it at all, this was actually my first proposal. The thing is that almost everybody who replied to the Discussion on internal wanted an OO API and no more procedural API. That's why I was thinking about multiple choices (the procedural API beeing one of them).
cmb
cmb
18:19
Actually, I would prefer a OO API, but given the discussion and the time pressure, it's clear now that we can't reach consensus with a discussion. And I don't like to force a decision by voting. But I also understand that many may like to have a OO API right away, so it might be best to go with an (STV) subvote with 4 choices.
@Danack begins circle of life theme song...
If you add a procedural version someone will probably come out with a OOP wrapper that becomes the de-facto standard. That said, I very much dislike the the idea of having a ton of extra global constants and would prefer the proposed class constants.
@Pierrick for the record, you have my commiserations. Although things like this should probably be done in PECL first before reaching php-core, for various reasons that is an awful and non-viable path of getting stuff done. And making it easier to use C code that isn't distributed with php-core is likely to be one of the long term goals the foundation should be working on.
Well there's nothing stopping the creation of an "Official PHP Library" via PECL that's managed by internals and that has regular updates with things like this.
@Danack I agree. If it wasn't something that would improve security I would not force to add this. Also I'm really considering on working on a more high level "url" extension that would wrap curl with a higher level api (and this will go to pecl).
18:27
URL manipulation, or CURL requests?
@MarkR URL request :-) A higher level Curl like API. And there will definitely be no reference to curl in the name...
I think those are pretty much already defined by PSR7 / PSR-18. Anything not using that is unlikely to gain any traction
If memory serves, guzzle holds something like 75% of the market with around half a billion downloads.
@MarkR I'm not sure, sometime you want to do simple call to URLs and stuff, and not necessarily want to use something like Guzzle, and the actual curl_ functions are not fun to use.
@Pierrick There's Guzzle-the-HTTP-client and Guzzle-the-PSR-7-library.
You can use the latter independently of the former.
@MarkR "that's managed by internals" - given how difficult it is to get internals to agree on anything, I think we need more diversity. And also, internals needs less things to discuss, not more.
18:34
@Pierrick Sometimes yes, but it's the exception, rather than the general rule. It may not have the impact it otherwise might if these existing tools were not ubiquitous.
@cmb I certainly would prefer a procedural API over an OO API I don't agree with. And I really believe that an API called "curl" should be "curl" (as I've also said on the list).
At least the former would be internally consistent with ext/curl.
19:02
I am laughing now because I thought this wouldn’t work but actually, it did. 3v4l.org/ZpJCG
> CONSTANT
actual values may vary
lol
19:31
I updated the RFC to reflect different implementations and add information on how the vote will work. wiki.php.net/rfc/curl-url-api . English is not my forte, so if someone want to clarify some things do not hesitate, let me know or feel free to update it if you have access !
19:42
@Pierrick I believe the RFC should highlight that the names of the flag constants for variant 3 and 4 are NOT the original names.
And for variant 1: Does it throw exceptions or not? I believe it does, but Exceptions are specifically mentioned the introduction of variant 2.
20:12
@Pierrick I fixed a bunch of typos and improved wording in a few places: wiki.php.net/rfc/…
@TimWolla OK I will clarify that. Thanks
@TheodoreBrown Thanks a lot !
@Pierrick I also fixed a couple places where the end of the sentence was missing: wiki.php.net/rfc/…
> Curl will use the given read-only object and will not change its contents.

I believe that changes the meaning. The object is not necessarily read-only. But it will not be modified by curl.
20:28
@TimWolla I will add clarification about the constant names. And exceptions are for each implementations. Exception is for all implementations. I'll clarify that in each section ! Thanks :-)
@TimWolla Hmm, if it's not read-only it seems confusing to mention this at all. Feel free to update the wording if you think of something better.
@TheodoreBrown There's a difference between the object being readonly ("immutable" which is only true in case 4) and the object used readonly. This becomes relevant, once you attach the object to the curl handle and modify it afterwards.
I'm not sure I'm a fan of the JSON indent RFC. An attribute that is only relevant when another flag is added is kinda weird, and on top of that it feels like there'd be more flexibility defining what the indent was e.g. "\t" or " "
20:46
@Pierrick You're welcome. One more thing: I also believe that __clone is missing in case 1 (curl_url_dup) and 3.
21:19
@TimWolla Fixed for __clone and constant renaming !
@Pierrick It would likely be more consistent to expose a curl_url_dup in (1). Otherwise you are not treating the CurlUrl object as an opaque handle.
@TimWolla __clone is just to specify that the object is clonable. Like CurlHandle is. I don't think that adding an other method is worth it.
I'm wondering whether we really should expose an url builder interface as part of curl. Nice that curl has such a thing, for C curl library users. But what's the motivation to tie that to curl in PHP? Why not have a proper URL class in PHP, independently of ext/curl?
In case of the procedural API I would then need to resort to clone to duplicate the handle, which effectively is no longer fully procedural. Not strong opinion on that one, though. Your call.
@bwoebi For security reasons you want the parser used in your application to be the same as the one used in your requester to make sure they don't handle same URL differently. blackhat.com/docs/us-17/thursday/…
21:31
I'm totally not opposed to ext/curl having a translation layer from a PHP native URL class, without going through the string url.
Just looked in the documentation for CURLU: everything.curl.dev/libcurl/url/redirect. That thing alone makes (3) and (4) a no for me, because using regular getters and setters imply that I get out whatever I pass in.
But ext/curl is not the only place where we use URLs. We use urls in all sorts of functions. An URL class from ext/curl could not be used e.g. in file_get_contents(). However, an URL class from ext/standard could be passed to ext/curl functions, which then do the translation into the curl URL API on the C layer.
What I wish for would be one URL API which can be used everywhere.
@bwoebi you're right that something like that would be nice, but 8.2 is feature freeze in two weeks, and I think it would be nice to have something to fix at least for curl since the API is already available.
@Pierrick I don't think that a looming feature freeze should ever be a motivation to shoehorn something into PHP. Actually, as far as I get, the reception to your RFC on ML has been quite controversial as well.
@Pierrick My suggestion would be to put your RFC on hold, explore how an URL class could be properly integrated into PHP as a whole, considering what the interoperability with various ext/ can be, and ultimately propose a well-rounded RFC - yes, for PHP 8.3 then.
@bwoebi it was quite controversial. The initial proposal was just to expose the Curl url api to follow how ext/curl is currently working but lot of people want to have a more high level OO api. And to be honest, I don't think that we'll ever have a consensus on what the ext/curl url design should be. If it was not improving security I would probably put this RFC on hold.
@bwoebi I feel that with just a simple thin wrapper on existing curl we could improve this area without affecting anything but curl, leaving space for any other amelioration such as the one you propose.
21:49
@Pierrick I think if we do anything right now, then it should be a simple thin wrapper, yes.
↑ that
@bwoebi I also strongly feel that and it was my original plan. But a lot of people reacted because the API wasn't OO (also we could have a thin OO wrapper which is the second implementation of the RFC). I feed that if I don't propose all the possible implementation I force the one I fill is the good one. And I'm only one voice.
However, I'm not yet persuaded in how much security benefit it actually brings. Especially, as libraries and frameworks will initially not be realistically able to use it, given that it's still very new (< 4 years old API). They will see that it will be problematic for quite a few users and thus avoid using it initially.
If you wanted to create a DTO at the same time to represent the result of a parsed URI, i'd vote in favour of that :-)
Now, however, if we had a proper URL class in PHP, we could check in ext/curl: if < 7.62, then stringify url and pass to curl, else translate to native curl url API. And it works seamlessly.
and everyone would have the security benefits from day one, the library targets the new PHP version.
22:03
I don't disagree with you and I think that would be a pretty nice addition. But the scope of this is bigger than just improving security for current curl users.
@Pierrick I'm mostly trying to say, that I'm not very happy with a specific API, specifically for ext/curl, which in essence could be already obsoleted by the next minor.
Thus I believe I'd rather oppose it, given that I consider the (effective) security it would bring right now very marginal.
@bwoebi I think we need a better alternative but there is no such thing yet.
@Pierrick yes. And I don't think it's great to squeeze a very shortterm improvement in.
22:18
@bwoebi From my POV, adding the first proposed implementation is harmless and can add benefice for user who may want use it. I'm just not comfortable changing all the RFC right now and say "this is the only implementation i'm comfortable to bring in 8.2 without changing everything so just say yes or no". Should I just do that ? Wouldn't it be to "selfish" ?
@Pierrick It's not selfish, you just have to expect more no votes :-)
Well I feel like a weathercock (this expression probably doesn't exists in english but in french it means that you change your mind every 2 seconds). This RFC gives me headache, thought it would be just like here are new curl functions, all the API is currently a thin wrapper, let's continue until we come with a newer full API.
Once something is added, it's very hard to un-add or change it. If you added something for 8.2 and it didn't work out, it would be 2025 before it could be removed, potentially 2030 depending on when it was agreed it wasn't working out.
yeah

« first day (4279 days earlier)      last day (894 days later) »