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

More Phabricator forge for git submit #866

Merged
merged 14 commits into from
Mar 25, 2023
Merged

More Phabricator forge for git submit #866

merged 14 commits into from
Mar 25, 2023

Conversation

arxanas
Copy link
Owner

@arxanas arxanas commented Mar 21, 2023

No description provided.

@arxanas arxanas force-pushed the arxanas/submit2 branch 7 times, most recently from fd5d9d4 to b3bd6b4 Compare March 24, 2023 07:34
arxanas added 14 commits March 24, 2023 03:35
…`post-commit` hook

If two commits aren't topologically related and then we simultaneously ask the `Dag` to process them, then it's not guaranteed which order they'll be visited. Then, later in testing, when we try to topologically sort a set of commits, the order of these two commits is non-deterministic.

In `hide.rs`, we get around this by using the `sorted_commit_set` function. However, this function isn't actually a sound sorting function (see comments internal to the function). In a future commit, we changed it to use `Dag::sort` instead, but this exposed the non-determinism above.

To fix this, we process the commit during the `post-commit` hook, to ensure that the commits are deterministically ordered (assuming that the commits were originally made in a deterministic order).
It seems that `sort` returns commits in the order of children first, which is the opposite of what we want in all cases. This commit fixes our wrapper around `.sort` to reverse the resulting commit list.
This is a long-outstanding bug: commits created during a rebase that is later aborted remain in the smartlog. To fix this, we defer registering the commits until the rebase has concluded.
The interface of `run_tests` was unsafe to use previously, since it could clobber working copy changes, and it would leave the commit graph in a semi-broken state (with rewritten commits showing in the smartlog).
…opy changes

Without this change, in addition to printing the normal helpful message, having uncommitted working copy changes would raise an unhandled error.
…de>`

For consistency with the other interface methods. We'll replace `()` with real data later.
It currently indiscriminately runs `arc diff`, even if the commits have not changed, which is suboptimal.
@arxanas arxanas merged commit 6b4dbcc into master Mar 25, 2023
@arxanas arxanas deleted the arxanas/submit2 branch March 25, 2023 01:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant