Sigh maybe update arcanist too. I ran arc lint before arc diff and it said no lints...
- Queries
- All Stories
- Search
- Advanced Search
- Transactions
- Transaction Logs
All Stories
Sep 4 2021
Try again but with arcanist on master
Rebased onto changes (no merge needed tho)
I'm working on testing out the issue before/after this change
I'll do that
I'd favor this change:
I hadn't considered that having this still use http requests could be done in parallel vs. the clustered running them in serial being faster (theoretically)
Yeah -- isReady() should really be called something else nowadays (it's, uh, "doTheThingThisFutureDoes()" pretty much), but it would be a backward-compatibility break and I shuffled enough stuff around in T11968 that I didn't want to press my luck.
Does that mean the ConduitClientFuture doesn't begin execution until resolve() is called or some other internal mechanism triggered by iterating a FutureIterator?
Right: creating a future doesn't start it (it doesn't make any network connections, launch any subprocesses, etc). Technically, you can start a future explicitly by calling $future->start(), but there's little/no reason to ever do so.
Even if we started futures on creation, the process is ultimately still single-threaded and we don't get any kind of I/O handling "for free" in PHP, so execution must pass into most Futures regularly to make meaningful progress on them: for example, if you just start() an HTTP future and don't return to it, it will probably get about as far as opening a network connection but not actually send any bytes over the wire, so it isn't really executing fully in parallel. Subprocess futures usually don't need as much upkeep as network futures, but they can still stall if the stderr buffer on the child process fills up and the parent process hasn't returned execution into the Future to read the buffer.
This might or might not be helpful, but the rough model here is that "hardpoints" are an open slot that some particular data may be loaded into. The physical analogy is a "hardpoint" on fighter aircraft where a particular missile or bomb may be attached.
oops
I looked at Future to see how isReady() plays into other parts and this looks good! Thank you!
Restore parallel future resolution
Does that mean the ConduitClientFuture doesn't begin execution until resolve() is called or some other internal mechanism triggered by iterating a FutureIterator?
The desired/intended approach is to implement ArcanistMercurialCommitSymbolCommitHardpointQuery, following ArcanistGitCommitSymbolCommitHardpointQuery. Alas, this is a significantly more complicated change (but might make life easier down the road).
I did have a suspicion about a missing hardpoint query for Mercurial but the only one I was tracing through was HARDPOINT_REVISIONREFS which I think maps to (or, I mapped to) ArcanistMercurialWorkingCopyRevisionHardpointQuery which looks to be appropriately implemented and mostly matches the Git variant. Though I don't have a solid mental model of "hardpoints".
Ah, that's helpful. I found a repro, it's specific to the behavior of the ImmediateFuture pathway for non-clustered resolution of Conduit "calls". This should force it from any configuration/state, I think:
When implemented correctly, a Future should not throw until it is resolved (at least, for any kind of "result of trying to resolve the future" error), so the Iterator loop should never throw. Some older Futures were less careful about this, but I believe major futures were pulled in line after T11968. From cursory inspection, I think the behavior of ConduitClientFuture is correct, and I can't immediately get the iterator loop to throw an exception (see T13666#256232 for my test case and observed behavior).
Does that mean the ConduitClientFuture doesn't begin execution until resolve() is called or some other internal mechanism triggered by iterating a FutureIterator? I will definitely add this back in.
I'm able to reproduce this but I'll investigate further
The desired/intended approach is to implement ArcanistMercurialCommitSymbolCommitHardpointQuery, following ArcanistGitCommitSymbolCommitHardpointQuery. Alas, this is a significantly more complicated change (but might make life easier down the road).
Thanks!
This construction:
I'll have some more detail in D21717 in a minute, but I can't immediately reproduce this, I think? If I intentionally break diffusion.historyquery like this:
Sep 3 2021
In D21715#276590, @cspeckmim wrote:I attempted to load the repository page and saw an exception due to a missing commit.
I don't think this is expected behavior but I'll make a new task for it since it's outside the scope of this change.
Created T13666
I attempted to load the repository page and saw an exception due to a missing commit.
I don't think this is expected behavior but I'll make a new task for it since it's outside the scope of this change.
In Mercurial, I'm not exactly sure how wide this window is. Since you normally can't strip commits in the remote by pushing (AFAIK?)
This is true, though it does change a bit if we enable evolve server-side. Then when commits/branches are pruned when pushing/pulling those obsolete markers will also be transferred, allowing similar disappearing commits (retrievable via —hidden). However I believe by default mercurial does not allow pruning public changesets (would require forcibly changing phases on client/server, or running a server/branch in non-publishing mode). Supporting evolve/topics server-side is probably large enough that it would be its own big project. So I think this assumption holds up well.
(D21715 is still a draft, but I left a couple of more specific comments there.)
Does this refer to when multiple dependent commits are pushed and then stripped from the on-disk state?
Another caveat here is that if you destroy commits directly in Phabricator's working copy in certain states, it's possible it won't be able to trace ancestry to find all the commits you destroyed in order to mark them unreachable. However, this window is narrow. For Git, you can bin/repository mark-reachable to rebuild reachability if you know you've done a bunch of dangerous mutations to the on-disk state.
Aug 20 2021
Thank you for figuring out the appropriate change!
Aug 19 2021
I think this is probably the cleanest fix, but I only tested event imports (which now appear to work). If you want to test Maniphest with subtypes configured (to make sure it doesn't break) and send me a revision with this change, I'll review it. Otherwise, I'll do that testing when I get a chance and land this if nothing crops up.
Aug 18 2021
Aug 17 2021
Aug 2 2021
I found this channel incredibly helpful for repairing/replacing components on boards:
Aug 1 2021
You can render remarkup using this rule without having a meaningful relative base URI, e.g. on some other object type or via remarkup.process in the API. This isn't necessarily a meaningful operation and the result may not be useful, but remarkup generally tries not to fail loudly: if you copy/paste a block of text from somewhere that happens to have some substrings which aren't valid, the desired behavior is generally for your text to be processed in some best-guess-at-sensible way -- and usually emitted unmodified (see also discussion in D21713).
This also fixes root document preview in lighttpd, which seems to have mandatory slash merging.
Jul 29 2021
- For now, just return the literal input if we fail to evaluate an expression.
When intent is ambiguous (the user might or might not be trying to invoke a Remarkup rule), I try to make the output of an "invalid" input exactly the same as the input, so (for example) copy/pasting text into Phabricator doesn't mangle it into a big blob of nonsense just because you happened to have some magic words in there.
I think _() is the pht() of Gnu "gettext". Modern programers may mostly be more familiar with Python than with gettext, of course.
Oh yea that's right. A long time ago I worked on a python web application that used this was probably confusing it.
I'll plan to get this on a test instance and get some screenshots
Try escaping the ${{{
I suspect escaping things in PHP will be pretty rare and that the "collides with PHP strings" downside will be very small.
Jul 28 2021
You can either escape ${{{strings.x.y}}} as \${{{strings.x.y}}} or suggest a different syntax for the "eval" rule --- I'm not married to ${{{...}}}.
I think this format makes sense to me. Nothing else really comes to mind. The squiggles are keeping consistency with other remarkup rules like figlet and cowsay. I think I've seen $ used in other places (and languages) with some relation to string substitution. I think python convention uses _, so another possibility could be _{{{...}}}.