Looks to be caused by this commit rPb75a4151c8996c5dfb1c8c14378fe3259666eac2

https://discourse.phabricator-community.org/t/daemons-tasks-crashing-in-a-loop-during-reindex/506

Running the reindex led to a couple crash looping tasks on my end:

@chad yes it is but I don’t want to disable it everywhere. It’s quite useful. Most websites just disable it selectively in places like search bars. I would also recommend disabling on form input like settings. Especially anywhere where phabricator autocompletes to some object like a project/user for access control etc. I hit this a lot while modifying many repositories visibility policies.

I can verify this fixes it! Thanks for this guys.

I’m fine waiting for the fix. Thanks @epriestley

Looks to be just the presence of the "?" in the text

"XXX://123456 XXX XXX XXX://123456 XXX XXX"

You and me both. I am super confused.

LOL @chad Literally reproduced this issue here by trying to paste the above line without the back ticks. It refuses to let me comment.

I'm not sure how without giving you the entire commit message which I cannot. I think key would be a having a commit with a line (probably the first one) that looks like this "XXX://123456 XXX XXX XXX? XXX://123456 XXX XXX XXX"

There also seems like there may be an issue with the current parsing logic since the "detected URI" has spaces which I do not think are valid in a uri and it should have been detected as two separate URI's with some stuff in the middle.

The URI that is ambiguous is a URI to a company App on MacOS. The only part of the URI that matters is the xxx://123456. The rest is just the title of the item referenced by this URI and this title contains a "?" which mixed with T12526 may be causing this issue. There may also be a place you now need to catch exceptions thanks to URI parsing logic changes. Just guessing here from what I can tell in the code.

This is pretty awesome improvement! Small feedback, the new scrollbar area is a bit hard to use on Safari with Apple's disappearing/reappearing scrollbars. They end up overlapping and it can make the Phabricator scrollbar area harder to use/click on etc. Would be nice if the scrollbar area shifted left while scrolling or if it were on the left side of the screen or some other magical solution.

Also, projects in this world are not allowed to explicitly manage their dependencies since other teams need to submit newer and newer versions and you NEED to build against whatever code they submit to the build. I think Phabricator is approaching build with the assumption that you can use a dependency manager like pip/composer/name-your-favorite-here and that a project can explicitly manage its dependencies and their versions which is just not true when building any OS.

But maybe the systems I'm aware of are all just toys for building web apps and all serious systems programmers use branch names to manage dependencies, but this is a very hard sell for me. Perhaps I'll have a change of heart eventually.

For reproduceable builds we only need to be able to reproduce official builds and for that the central build team stores all the tags and projects submitted to each build for a given train.

Commits may be on multiple releases but issues are reported from a platform that is associated with a particular sdk/release. Engineers work backwards from there. In addition, for us every commit message has a tracking ticket in it as well and the official build system updates those tickets with information on where it got incorporated when etc.

For someone debugging on their desk they will normally build with the latest sdk for that release or if they suspect some issue related to build then they will use the sdk generated from when that commit was built in the official (internal) release.

Dependency information between projects in the real build team is kept for the general project a needs b and c and when that new dependency is introduced. They can update this before a build by looking for any new dylib/static lib/header dependencies that were not there before and looking up what project produces that file. They do not have a version level dependency since teams don't always coordinate like that unless an API breakage is coming. Even then the build team may roll back a projects submission if it causes other teams to fail to build in the official nightlies.

We already build reviews after they land since it's now obvious to our build system what it needs to do.

There's two parts to the process (being transparent here). For official builds we submit tags to the "real" build team for a given release line. The submission info contains any new dependency information etc. Teams tend to generate these tags from branches with names matching the release code name.

We adopted this convention (instead of a file containing the mappings or settings etc) because we have many many major versions of the OS we have to support with fixes and it's for all developers to just create branches with a specific scheme instead of constantly updating a file everywhere or having to look in a file to know where this code will go.

@epriestley calling it a odd use case is a little rough. It's quiet common in the systems programming world. Especially when you are building operating systems. The main reason for the branch naming scheme is that we have multiple sdks being produced (one for each version of the OS). There are multiple versions being developed for release (master may have moved on to far future work and it's sdk is no longer compatible when building another branch that branched because a release is coming soon). Even the sdk is being used is built iteratively from the results of other projects completing their build.

ah poop worth a shot.

@epriestley Since you added a new "Waiting on other reviewers" bucket (D17425) is it possible to have that bucket cover this case as well? I believe that bucket makes a lot more sense for this case as well then the current "Must Review".

@epriestley I was just jumping on here to provide some simple examples and reasoning from my experience on our internal bug tracker (we don't use Maniphest). Note this isn't my bug. Just trying to help out someone by giving an example for reasons they could want this. I'm sure your response was well intentioned but does come off a bit user hostile. For instance, nowhere did anyone mention wasting time by having a constructive conversation about this. It gives the impression that you guys consider having to deal with the community is a waste of your time. And no one is arguing against the root problem concept.

Wouldn't that split up important data? A project manager will normally always be looking at the project board and moving tasks around there. Would it be cumbersome to have to go to some other application in order to do a query to figure out the relations on this one object? Not sure how you guys are thinking about Facts but going somewhere else for that info might be too cumbersome to do often. Especially if I'm jumping around a group of tasks in projects already. I would want that information presented there as well.

As to why it's important I think dependes on the style of project management being employed. Some places keep track of the dupe count as a way to gauge how often an issue is being seen or how many separate requests for a feature have shown up. A task with a lot of dupes might end up getting higher priority because of the number of dupes.

Maybe a more useful form of this would be some form of being able to view "how all related objects are related to this one". You can currently see related objects but you cannot easily tell the nature of that relationship for some of them. The task graph solves part of this for dependencies but not for other relation types.

T11050 seems to be describing a different issue. I don't mind having the review in my queue just not in the must review section.

Yeah sorry I was missin your step 2a from above. I guess this behavior just feels weird since it's saying specifically that I must review. As a user I intuitively think of must review as something that I have personally blocked. For user B to see must review makes sense. But I don't think the same applies to every other member of the project. It's ok for the review to remain in my queue under needs review. Just not all of a sudden upgraded to must review.

In T12027#209445, @epriestley wrote:

D17283 allows you to double-click the D1234 crumb to select it, since it's no longer a link. I don't know if double-clicks are too complicated or not.

Some users really do not like command line. Like really. 😂

Just for my own clarification: these are technical users using Git to review changes to code, who don't enable "Full website address" and don't like the command line?

One general improvement we could make is to have arc patch without arguments offer you open, non-authored revisions in the same repository, using the new numerical menu selector from T10895. That might moot this, and I can't immediately imagine any other default behaviors for arc patch with no arguments (other than showing you how to provide arguments, as it currently does, but we could retain this text in an abbreviated form).

This text is larger than the old arc patch text was, isn't it? It's also copyable from the URL bar. And surely text from a hypothetical arc list-stuff-I-am-reviewing isn't any easier to copy/paste than this text?

Moving over comment from T12179

@chad yeah sorry for the noise I will go ahead and move it as a comment over there.

@avivey my local repo does not have the metadata file but some of the commits do have the git-svn-id in the commit message. I think once we make enough commits on this repo this problem should probably go away but this particular repo doesn't receive commits very often. I would rather not fork arc.

We keep hitting the same thing where a conduit reports an svn commit for "sourceControlBaseRevision" on a repo that is git (was originally svn and was transferred to git).

@chad Ah bummer. Is there an official channel you guys recommend for feedback of this type?

Thanks for the info!

In T10700#196511, @chad wrote:

It's a solution you can have in production today, at no cost. Evan noted above we have no immediate plans to remove/replace.

@chad is this really a solution? The events documentation clearly states

In T11716#196494, @chad wrote:

I think you can lock this yourself now by using your own AuthAdapter.

In T11716#196493, @chad wrote:

some users decided to use a different username on phabricator

How did they change their username?

@chad I updated the description. I hope this is more of a root problem.

Ah yes I forgot about the fun of internationalization. Btw those fragments are correct 👍🏻 (I speak Spanish).

The translation.override settings seems to be the easiest path forward. I definitely don't want to be modifying the source if I can help it.

Essentially I would like to customize the "Login or register with LDAP" here to be "Login or register with OpenDirectory"

@epriestley thanks for the extra context. Makes sense now haha. Sad to see this get deprioritized though.

@epriestley can you explain how this was building up toward T10939? That one deals with automatically adding reviewers while this enables project membership (and hence policies) to be based on a predefined source of truth at most companies (LDAP).

@epriestley Thats good to know. Also, will this change include formatting the Change Details section of emails? That one is currently a blob of visually unparseable text even in HTML emails.

@epriestley This is kind of random but this looks like it will look really cool in email. Is there any chance of bringing these snippets into the discussion view on a review instead of just the links to the inline comments? Kind of like how github does this?

@epriestley can this information (branched from master) be exposed via conduit as well? Seems like a simple addition to differential.query

Only due to the fact that it failed 250 times and got removed by Phabricator.... I suspect it will happen again the next time that file changes.

sadly I can't run this

bin/worker execute --id <id>

since the task has been archived

I have metamta.differential.inline-patches set to 100
metamta.email-body-limit is set to the Global Default 524288

In T9803#144799, @epriestley wrote:

The default value of metamta.email-body-limit is 512KB. Does your server reject 512KB messages, or have you raised this limit, or do you expect 512KB messages to send cleanly?

I just noticed the Diff UI also attempts to show this file.

The particular file in question was a .yuv file which is used for video data format file.

I ran this command out of curiosity:

As a side work around, is there anyway to "complete" this task or kick it off the queue? It really is not that important if one email fails to be sent.

Ah that was it. I increased that and also had to increase the innodb_log_file_size.