A component's build progress reads under its buttons, and an unrelated history can be overridden
Two things, both about a card being able to act on what it says. The progress bar, its counts and the last line a component printed now sit below that component's buttons rather than above them. A bar reports on the press that started it, so it reads in the order it happened -- and above, it pushed the buttons down the moment a build began, moving the row somebody had just pressed out from under their finger. A pull that cannot fast-forward because the checkout shares no history with its upstream now offers a way past it. There is no fast-forward between two unrelated histories and there never will be, so the card was one that could never be pulled again, with the only remedy on the build machine -- exactly where the person holding the phone isn't. The card reports the failure as before and a dialog offers a forced pull, naming the branch and the upstream it is about to overwrite; confirming sends ?force=true, which resets onto the upstream instead of merging. Whether it *is* that failure is decided structurally, by `git merge-base` finding no common ancestor, rather than by matching what git printed: those messages are translated, and a button that appeared only on an English build machine would be worse than no button. It travels to the phone as its own field for the same reason. The dirty-tree refusal stays in front of it, so a forced pull can only ever discard something that was committed, and a merely diverged history is not offered it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
b0e83059a3
commit
db47972a25
6 files changed
+306
-29
No files matched your search
@@ -256,6 +256,23 @@ mutable at runtime from the phone.
|
|||||||
remote deliberately, and a card that mentions it every time is nagging
|
remote deliberately, and a card that mentions it every time is nagging
|
||||||
about a choice somebody made.
|
about a choice somebody made.
|
||||||
|
|
||||||
|
- **Unrelated histories are the one pull failure the phone may override.**
|
||||||
|
A checkout sharing no commit with its upstream has no fast-forward and
|
||||||
|
never will, so with nothing offered the card is one that can never be
|
||||||
|
pulled again -- and the way out is on the build machine, which is
|
||||||
|
exactly where the person holding the phone isn't. So `?force=true` on
|
||||||
|
the pull route resets onto the upstream instead of merging, behind a
|
||||||
|
dialog that names the branch it is about to overwrite (the only place
|
||||||
|
that is seen before it goes). Whether this *is* that failure is decided
|
||||||
|
structurally -- `git merge-base` finding no common ancestor -- and never
|
||||||
|
by matching what git printed: those messages are translated, and a
|
||||||
|
button that appeared only on an English build machine is worse than no
|
||||||
|
button. It travels as `PullError::unrelated_histories` rather than
|
||||||
|
inside the message for the same reason. The dirty-tree refusal stays in
|
||||||
|
front of it, so a forced pull can only ever discard something that was
|
||||||
|
committed, and a merely *diverged* history is not offered it -- there is
|
||||||
|
something better than throwing that away.
|
||||||
|
|
||||||
- **The branch line is how a failed remote check is visible at all.** With
|
- **The branch line is how a failed remote check is visible at all.** With
|
||||||
it gone, `newCommits` stays false and the card reads as an app with no
|
it gone, `newCommits` stays false and the card reads as an app with no
|
||||||
updates -- the same silent failure the self-entry note below describes.
|
updates -- the same silent failure the self-entry note below describes.
|
||||||
|
|||||||
@@ -17,6 +17,13 @@ data class BuildStatus(
|
|||||||
val stale: Boolean,
|
val stale: Boolean,
|
||||||
val building: Boolean,
|
val building: Boolean,
|
||||||
val error: String?,
|
val error: String?,
|
||||||
|
// The failure above was a pull with no fast-forward to make, because
|
||||||
|
// the checkout on the build machine shares no commit with its
|
||||||
|
// upstream. The one failure this app offers a way past, and it is
|
||||||
|
// said as a fact of its own rather than read out of [error]: git's
|
||||||
|
// wording is translated, so a button that matched on it would appear
|
||||||
|
// only on an English build machine.
|
||||||
|
val unrelatedHistories: Boolean,
|
||||||
// What the whole *project* is doing ("fetching", "pulling"), and how
|
// What the whole *project* is doing ("fetching", "pulling"), and how
|
||||||
// long this run has taken. Work belonging to one component is in
|
// long this run has taken. Work belonging to one component is in
|
||||||
// [components] instead, because that is where it is drawn.
|
// [components] instead, because that is where it is drawn.
|
||||||
@@ -66,6 +73,7 @@ private fun requestBuildStatus(path: String, method: String): BuildStatus =
|
|||||||
stale = json.getBoolean("stale"),
|
stale = json.getBoolean("stale"),
|
||||||
building = json.getBoolean("building"),
|
building = json.getBoolean("building"),
|
||||||
error = if (json.isNull("error")) null else json.getString("error"),
|
error = if (json.isNull("error")) null else json.getString("error"),
|
||||||
|
unrelatedHistories = json.optBoolean("unrelatedHistories", false),
|
||||||
phase = if (json.isNull("phase")) null else json.optString("phase").ifEmpty { null },
|
phase = if (json.isNull("phase")) null else json.optString("phase").ifEmpty { null },
|
||||||
elapsedMs = json.optLong("elapsedMs", 0),
|
elapsedMs = json.optLong("elapsedMs", 0),
|
||||||
components =
|
components =
|
||||||
@@ -93,7 +101,16 @@ private fun requestBuildStatus(path: String, method: String): BuildStatus =
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
fun pullAndBuild(key: String): BuildStatus = requestBuildStatus("/apps/$key/pull", "POST")
|
/**
|
||||||
|
* Fetches and fast-forwards the checkout on the build machine, then builds it.
|
||||||
|
*
|
||||||
|
* [force] resets the checkout onto its upstream instead, throwing away every commit it has that the
|
||||||
|
* remote doesn't. Only ever sent after a plain pull has come back saying the two histories are
|
||||||
|
* unrelated, and only after the dialog that says so has been confirmed -- there is no fast-forward
|
||||||
|
* for that case ever, so the alternative is a project that can never be pulled again.
|
||||||
|
*/
|
||||||
|
fun pullAndBuild(key: String, force: Boolean = false): BuildStatus =
|
||||||
|
requestBuildStatus("/apps/$key/pull" + if (force) "?force=true" else "", "POST")
|
||||||
|
|
||||||
fun prepareBuild(key: String): BuildStatus = requestBuildStatus("/apps/$key/prepare", "POST")
|
fun prepareBuild(key: String): BuildStatus = requestBuildStatus("/apps/$key/prepare", "POST")
|
||||||
|
|
||||||
|
|||||||
@@ -428,6 +428,11 @@ private fun AppListScreen(
|
|||||||
// local storage rather than the manifest. Held as state so picking one
|
// local storage rather than the manifest. Held as state so picking one
|
||||||
// redraws the card without a round trip.
|
// redraws the card without a round trip.
|
||||||
var chosenVariants by remember { mutableStateOf<Map<String, String>>(emptyMap()) }
|
var chosenVariants by remember { mutableStateOf<Map<String, String>>(emptyMap()) }
|
||||||
|
// The project whose pull came back saying its checkout and its remote
|
||||||
|
// share no history, waiting on an answer about throwing that history
|
||||||
|
// away. One slot rather than one per card: it is a modal, so only one
|
||||||
|
// can be open, and the entry in it says which card asked.
|
||||||
|
var forcePull by remember { mutableStateOf<ManifestEntry?>(null) }
|
||||||
// Which component of which project has a service action in flight.
|
// Which component of which project has a service action in flight.
|
||||||
// Per project, because that is the granularity of a card, and the row
|
// Per project, because that is the granularity of a card, and the row
|
||||||
// that is busy is the one that shows it.
|
// that is busy is the one that shows it.
|
||||||
@@ -625,6 +630,10 @@ private fun AppListScreen(
|
|||||||
val failure = status.error
|
val failure = status.error
|
||||||
if (failure != null) {
|
if (failure != null) {
|
||||||
cardStates = cardStates + (entry.key to CardState.Error(failure, retryPull))
|
cardStates = cardStates + (entry.key to CardState.Error(failure, retryPull))
|
||||||
|
// The card keeps the message either way -- the dialog is
|
||||||
|
// dismissible, and a failure that vanished with it would
|
||||||
|
// leave the card looking as though nothing had happened.
|
||||||
|
if (status.unrelatedHistories) forcePull = entry
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
// The APK's mtime is what decides "update available", so the
|
// The APK's mtime is what decides "update available", so the
|
||||||
@@ -669,13 +678,18 @@ private fun AppListScreen(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Pull acts on the build machine: fetch, fast-forward, rebuild. */
|
/**
|
||||||
fun startPull(entry: ManifestEntry) {
|
* Pull acts on the build machine: fetch, fast-forward, rebuild.
|
||||||
|
*
|
||||||
|
* [force] is the answer to the dialog below, and nothing else ever passes it: it abandons
|
||||||
|
* whatever history that checkout has of its own.
|
||||||
|
*/
|
||||||
|
fun startPull(entry: ManifestEntry, force: Boolean = false) {
|
||||||
scope.launch {
|
scope.launch {
|
||||||
followBuild(
|
followBuild(
|
||||||
entry,
|
entry,
|
||||||
retryPull = true,
|
retryPull = true,
|
||||||
start = { pullAndBuild(entry.key) },
|
start = { pullAndBuild(entry.key, force) },
|
||||||
progress = { CardState.Pulling(it) },
|
progress = { CardState.Pulling(it) },
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
@@ -1073,8 +1087,66 @@ private fun AppListScreen(
|
|||||||
) {
|
) {
|
||||||
Text(PLUS_GLYPH, fontFamily = NerdIcons, fontSize = 22.sp)
|
Text(PLUS_GLYPH, fontFamily = NerdIcons, fontSize = 22.sp)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
forcePull?.let { entry ->
|
||||||
|
ForcePullDialog(
|
||||||
|
entry = entry,
|
||||||
|
onDismiss = { forcePull = null },
|
||||||
|
onForce = {
|
||||||
|
forcePull = null
|
||||||
|
startPull(entry, force = true)
|
||||||
|
},
|
||||||
|
)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Offered when a pull comes back saying the checkout and its remote share no history at all.
|
||||||
|
*
|
||||||
|
* There is no fast-forward between two unrelated histories and there never will be, so without this
|
||||||
|
* the card is one that can never be pulled again — and the way out is on the build machine, which
|
||||||
|
* is exactly where the person holding the phone isn't. So the capability is shown rather than
|
||||||
|
* withheld, with what it costs said in front of it: the alternative to a destructive button here is
|
||||||
|
* not safety, it is a dead card.
|
||||||
|
*
|
||||||
|
* The branch and its upstream are named because they are what is about to be overwritten, and this
|
||||||
|
* is the only place they are seen before it happens.
|
||||||
|
*/
|
||||||
|
@Composable
|
||||||
|
private fun ForcePullDialog(entry: ManifestEntry, onDismiss: () -> Unit, onForce: () -> Unit) {
|
||||||
|
// Named where they are known. Pull is only offered for a checkout
|
||||||
|
// with an upstream, so the fallbacks are for a list that has moved on
|
||||||
|
// since the failure rather than for the ordinary case.
|
||||||
|
val branch = entry.git?.branch ?: "the branch"
|
||||||
|
val upstream = entry.git?.upstream ?: "the remote"
|
||||||
|
AlertDialog(
|
||||||
|
onDismissRequest = onDismiss,
|
||||||
|
title = { Text("${entry.label} shares no history with $upstream") },
|
||||||
|
text = {
|
||||||
|
Text(
|
||||||
|
"Its checkout on the build machine has no commit in common with $upstream, so " +
|
||||||
|
"there is no fast-forward to make and Pull can go no further.\n\n" +
|
||||||
|
"Forcing resets $branch onto $upstream. Every commit the build machine has " +
|
||||||
|
"that the remote doesn't is abandoned -- they stay in that checkout's " +
|
||||||
|
"reflog, but nothing on this phone will bring them back.\n\n" +
|
||||||
|
"Uncommitted changes are not touched: a pull refuses over those before it " +
|
||||||
|
"ever gets this far."
|
||||||
|
)
|
||||||
|
},
|
||||||
|
confirmButton = {
|
||||||
|
TextButton(
|
||||||
|
// The red every control that takes something away wears,
|
||||||
|
// so this doesn't read as the ordinary way past a message.
|
||||||
|
colors = ActionTone.Destructive.colors(),
|
||||||
|
onClick = onForce,
|
||||||
|
) {
|
||||||
|
Text("Force pull")
|
||||||
|
}
|
||||||
|
},
|
||||||
|
dismissButton = { TextButton(onClick = onDismiss) { Text("Cancel") } },
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
@Composable
|
@Composable
|
||||||
private fun AppCard(
|
private fun AppCard(
|
||||||
@@ -2006,13 +2078,6 @@ private fun ComponentCard(
|
|||||||
// together -- controls belong to an APK, service buttons to a
|
// together -- controls belong to an APK, service buttons to a
|
||||||
// server -- so each gets the card's full width instead of
|
// server -- so each gets the card's full width instead of
|
||||||
// sharing a row. A progress bar in particular wants all of it.
|
// sharing a row. A progress bar in particular wants all of it.
|
||||||
// Before the controls, and inside this component's own card:
|
|
||||||
// what is happening to *this* component belongs with it, not
|
|
||||||
// under the project where it could be any of them.
|
|
||||||
build?.let {
|
|
||||||
Spacer(Modifier.height(6.dp))
|
|
||||||
ComponentBuildProgress(it)
|
|
||||||
}
|
|
||||||
controls()
|
controls()
|
||||||
// Nothing to offer until its script has been asked: the buttons
|
// Nothing to offer until its script has been asked: the buttons
|
||||||
// depend on the answer, and guessing which to show would mean
|
// depend on the answer, and guessing which to show would mean
|
||||||
@@ -2073,6 +2138,16 @@ private fun ComponentCard(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Under the buttons, and inside this component's own card. It
|
||||||
|
// reports on the press that started it, so it reads in the
|
||||||
|
// order it happened -- and putting it above meant the buttons
|
||||||
|
// moved down the moment a build began, so the row somebody had
|
||||||
|
// just pressed slid out from under their finger.
|
||||||
|
build?.let {
|
||||||
|
Spacer(Modifier.height(6.dp))
|
||||||
|
ComponentBuildProgress(it)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -102,6 +102,12 @@ struct Inner {
|
|||||||
/// where this is being driven from and usually has no access to the
|
/// where this is being driven from and usually has no access to the
|
||||||
/// server's log.
|
/// server's log.
|
||||||
error: Option<String>,
|
error: Option<String>,
|
||||||
|
/// Whether that failure was a pull with no fast-forward to make,
|
||||||
|
/// which is the one the phone can offer a way past. Beside the
|
||||||
|
/// message rather than inside it because the phone acts on it: a
|
||||||
|
/// button that appears only when git happened to phrase itself a
|
||||||
|
/// certain way is a button nobody can rely on.
|
||||||
|
unrelated_histories: bool,
|
||||||
/// The component whose step produced that failure.
|
/// The component whose step produced that failure.
|
||||||
///
|
///
|
||||||
/// Kept beside the message so the card can open the right log without
|
/// Kept beside the message so the card can open the right log without
|
||||||
@@ -180,6 +186,10 @@ pub struct BuildStatus {
|
|||||||
pub stale: bool,
|
pub stale: bool,
|
||||||
pub building: bool,
|
pub building: bool,
|
||||||
pub error: Option<String>,
|
pub error: Option<String>,
|
||||||
|
/// That failure was a pull the checkout has no fast-forward for,
|
||||||
|
/// because it shares no history with its upstream. The phone offers
|
||||||
|
/// the forced pull for this and nothing else.
|
||||||
|
pub unrelated_histories: bool,
|
||||||
/// What the whole project is doing -- fetching, pulling -- absent
|
/// What the whole project is doing -- fetching, pulling -- absent
|
||||||
/// between runs and while the work belongs to a component instead.
|
/// between runs and while the work belongs to a component instead.
|
||||||
pub phase: Option<String>,
|
pub phase: Option<String>,
|
||||||
@@ -387,6 +397,7 @@ impl BuildState {
|
|||||||
inner.building = true;
|
inner.building = true;
|
||||||
inner.error = None;
|
inner.error = None;
|
||||||
inner.failed = None;
|
inner.failed = None;
|
||||||
|
inner.unrelated_histories = false;
|
||||||
inner.started = Some(Instant::now());
|
inner.started = Some(Instant::now());
|
||||||
inner.runs.clear();
|
inner.runs.clear();
|
||||||
}
|
}
|
||||||
@@ -509,8 +520,14 @@ impl BuildState {
|
|||||||
/// whose new commits changed the declaration built anyway -- the card
|
/// whose new commits changed the declaration built anyway -- the card
|
||||||
/// went off and updated the app instead of stopping to ask, which is
|
/// went off and updated the app instead of stopping to ask, which is
|
||||||
/// the single case this gate exists for.
|
/// the single case this gate exists for.
|
||||||
|
/// `force` abandons this checkout's own history in favour of the
|
||||||
|
/// upstream's, and is only ever pressed after a pull has already
|
||||||
|
/// reported that the two are unrelated (`git::PullError`). It is a
|
||||||
|
/// parameter rather than something decided here because it is a
|
||||||
|
/// person's answer to that report, not a state of the repository.
|
||||||
pub fn pull_and_build(
|
pub fn pull_and_build(
|
||||||
self: &Arc<Self>,
|
self: &Arc<Self>,
|
||||||
|
force: bool,
|
||||||
may_build: impl Fn() -> bool + Send + 'static,
|
may_build: impl Fn() -> bool + Send + 'static,
|
||||||
record: RecordBuilt,
|
record: RecordBuilt,
|
||||||
) {
|
) {
|
||||||
@@ -522,18 +539,18 @@ impl BuildState {
|
|||||||
inner.building = true;
|
inner.building = true;
|
||||||
inner.error = None;
|
inner.error = None;
|
||||||
inner.failed = None;
|
inner.failed = None;
|
||||||
|
inner.unrelated_histories = false;
|
||||||
inner.started = Some(Instant::now());
|
inner.started = Some(Instant::now());
|
||||||
inner.runs.clear();
|
inner.runs.clear();
|
||||||
}
|
}
|
||||||
|
|
||||||
let this = Arc::clone(self);
|
let this = Arc::clone(self);
|
||||||
tokio::task::spawn_blocking(move || {
|
tokio::task::spawn_blocking(move || {
|
||||||
let outcome = this.pull();
|
let outcome = this.pull(force);
|
||||||
match outcome {
|
match outcome {
|
||||||
Err(message) => {
|
Err(error) => {
|
||||||
tracing::error!("pull failed: {message}");
|
tracing::error!("pull failed: {}", error.message);
|
||||||
// No component: a pull fails before the walk starts.
|
this.fail_pull(error);
|
||||||
this.finish(Some(message), None);
|
|
||||||
}
|
}
|
||||||
Ok(pulled) => {
|
Ok(pulled) => {
|
||||||
// Nothing configured to build, or nothing allowed to:
|
// Nothing configured to build, or nothing allowed to:
|
||||||
@@ -553,7 +570,7 @@ impl BuildState {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/// Fetches and fast-forwards, reporting whether anything arrived.
|
/// Fetches and fast-forwards, reporting whether anything arrived.
|
||||||
fn pull(&self) -> Result<bool, String> {
|
fn pull(&self, force: bool) -> Result<bool, crate::git::PullError> {
|
||||||
self.begin_project("fetching");
|
self.begin_project("fetching");
|
||||||
crate::git::fetch(&self.project_path, self.git_ipv4)?;
|
crate::git::fetch(&self.project_path, self.git_ipv4)?;
|
||||||
// Counted after the fetch, from refs now on disk: this is the one
|
// Counted after the fetch, from refs now on disk: this is the one
|
||||||
@@ -563,7 +580,7 @@ impl BuildState {
|
|||||||
return Ok(false);
|
return Ok(false);
|
||||||
}
|
}
|
||||||
self.begin_project("pulling");
|
self.begin_project("pulling");
|
||||||
crate::git::pull(&self.project_path, self.git_ipv4)?;
|
crate::git::pull(&self.project_path, self.git_ipv4, force)?;
|
||||||
tracing::info!(
|
tracing::info!(
|
||||||
"pulled {behind} commit(s) into {}",
|
"pulled {behind} commit(s) into {}",
|
||||||
self.project_path.display()
|
self.project_path.display()
|
||||||
@@ -990,6 +1007,19 @@ impl BuildState {
|
|||||||
inner.failed = failed;
|
inner.failed = failed;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Finishes a run that never got past its pull.
|
||||||
|
///
|
||||||
|
/// Separate from [`Self::finish`] only because a pull failure carries
|
||||||
|
/// the one thing a build failure cannot: whether abandoning this
|
||||||
|
/// checkout's own history would clear it. Cleared where its siblings
|
||||||
|
/// are, at the start of every run, so this is the only thing that can
|
||||||
|
/// ever make it true.
|
||||||
|
fn fail_pull(&self, error: crate::git::PullError) {
|
||||||
|
self.inner.lock().unwrap().unrelated_histories = error.unrelated_histories;
|
||||||
|
// No component: a pull fails before the walk starts.
|
||||||
|
self.finish(Some(error.message), None);
|
||||||
|
}
|
||||||
|
|
||||||
/// Records how far the running command says it has got. Replaces the
|
/// Records how far the running command says it has got. Replaces the
|
||||||
/// previous count rather than accumulating: the command reports its
|
/// previous count rather than accumulating: the command reports its
|
||||||
/// own total, and `begin` clears this so one component's count is
|
/// own total, and `begin` clears this so one component's count is
|
||||||
@@ -1041,6 +1071,7 @@ impl BuildState {
|
|||||||
stale,
|
stale,
|
||||||
building: inner.building,
|
building: inner.building,
|
||||||
error: inner.error.clone(),
|
error: inner.error.clone(),
|
||||||
|
unrelated_histories: inner.unrelated_histories,
|
||||||
phase: inner.phase.clone(),
|
phase: inner.phase.clone(),
|
||||||
elapsed_ms: inner
|
elapsed_ms: inner
|
||||||
.started
|
.started
|
||||||
@@ -1230,6 +1261,7 @@ mod tests {
|
|||||||
|
|
||||||
let project = clone.clone();
|
let project = clone.clone();
|
||||||
state.pull_and_build(
|
state.pull_and_build(
|
||||||
|
false,
|
||||||
move || crate::config::project_config(&project).matches_accepted(&accepted, None),
|
move || crate::config::project_config(&project).matches_accepted(&accepted, None),
|
||||||
Arc::new(|_: &str, _: String| {}),
|
Arc::new(|_: &str, _: String| {}),
|
||||||
);
|
);
|
||||||
|
|||||||
+131
-11
@@ -246,28 +246,70 @@ fn run_bounded(project: &Path, ipv4: bool, args: &[&str]) -> Result<String, Stri
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// What stopped a pull, and whether there is anything the phone can do
|
||||||
|
/// about it.
|
||||||
|
///
|
||||||
|
/// Every failure here is a message and nothing else, with one exception: a
|
||||||
|
/// branch and its upstream that share no commit have no fast-forward
|
||||||
|
/// between them and never will, so the only thing that would ever move
|
||||||
|
/// that checkout is discarding one side's history. That is destructive
|
||||||
|
/// enough that nobody should do it by inference, so it travels as a fact
|
||||||
|
/// of its own rather than as a message the phone would have to match on --
|
||||||
|
/// git's is translated, and a button that appears only in English is
|
||||||
|
/// worse than no button.
|
||||||
|
#[derive(Debug)]
|
||||||
|
pub struct PullError {
|
||||||
|
pub message: String,
|
||||||
|
pub unrelated_histories: bool,
|
||||||
|
}
|
||||||
|
|
||||||
|
impl From<String> for PullError {
|
||||||
|
fn from(message: String) -> Self {
|
||||||
|
Self {
|
||||||
|
message,
|
||||||
|
unrelated_histories: false,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// Fast-forwards the current branch onto its upstream.
|
/// Fast-forwards the current branch onto its upstream.
|
||||||
///
|
///
|
||||||
/// `--ff-only` deliberately: a merge or a rebase can conflict, and
|
/// `--ff-only` deliberately: a merge or a rebase can conflict, and
|
||||||
/// resolving a conflict is not something to start from a phone with no way
|
/// resolving a conflict is not something to start from a phone with no way
|
||||||
/// to finish it. Refusing to touch a dirty tree is the same reasoning --
|
/// to finish it. Refusing to touch a dirty tree is the same reasoning --
|
||||||
/// the failure is reported and the working tree is left exactly as it was.
|
/// the failure is reported and the working tree is left exactly as it was.
|
||||||
pub fn pull(project: &Path, ipv4: bool) -> Result<(), String> {
|
///
|
||||||
|
/// `force` replaces the fast-forward with a hard reset onto the upstream,
|
||||||
|
/// which is what somebody presses after being told the histories are
|
||||||
|
/// unrelated: it throws away every commit this checkout has that the
|
||||||
|
/// remote does not. The dirty check stays in front of it, so what it
|
||||||
|
/// discards is always something that was committed -- still in the
|
||||||
|
/// reflog on the build machine -- and never somebody's uncommitted work.
|
||||||
|
pub fn pull(project: &Path, ipv4: bool, force: bool) -> Result<(), PullError> {
|
||||||
let status = status(project).ok_or_else(|| "not a git repository".to_string())?;
|
let status = status(project).ok_or_else(|| "not a git repository".to_string())?;
|
||||||
if status.upstream.is_none() {
|
if status.upstream.is_none() {
|
||||||
return Err(format!(
|
return Err(format!(
|
||||||
"branch {} tracks no upstream, so there is nothing to pull",
|
"branch {} tracks no upstream, so there is nothing to pull",
|
||||||
status.branch
|
status.branch
|
||||||
));
|
)
|
||||||
|
.into());
|
||||||
}
|
}
|
||||||
if status.dirty {
|
if status.dirty {
|
||||||
return Err(format!(
|
return Err(format!(
|
||||||
"{} has uncommitted changes -- refusing to pull over them. Commit or stash on the \
|
"{} has uncommitted changes -- refusing to pull over them. Commit or stash on the \
|
||||||
build machine first.",
|
build machine first.",
|
||||||
project.display()
|
project.display()
|
||||||
));
|
)
|
||||||
|
.into());
|
||||||
|
}
|
||||||
|
if force {
|
||||||
|
git(project, &["reset", "--hard", "@{u}"])?;
|
||||||
|
} else if let Err(message) = git(project, &["merge", "--ff-only", "@{u}"]) {
|
||||||
|
return Err(PullError {
|
||||||
|
unrelated_histories: unrelated_to_upstream(project),
|
||||||
|
message,
|
||||||
|
});
|
||||||
}
|
}
|
||||||
git(project, &["merge", "--ff-only", "@{u}"])?;
|
|
||||||
|
|
||||||
// A merge moves the *pointer* a submodule is recorded at without
|
// A merge moves the *pointer* a submodule is recorded at without
|
||||||
// touching its working tree, so a pull that updated one leaves the
|
// touching its working tree, so a pull that updated one leaves the
|
||||||
@@ -298,6 +340,22 @@ pub fn pull(project: &Path, ipv4: bool) -> Result<(), String> {
|
|||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Whether HEAD and its upstream share any commit at all.
|
||||||
|
///
|
||||||
|
/// Asked of the object store rather than read off what git printed:
|
||||||
|
/// "refusing to merge unrelated histories" is a translated string, so
|
||||||
|
/// matching it would work on this machine and quietly stop working on one
|
||||||
|
/// running in another language. `merge-base` fails, printing nothing,
|
||||||
|
/// exactly when there is no common ancestor.
|
||||||
|
///
|
||||||
|
/// Only asked once a fast-forward has already failed, which is what keeps
|
||||||
|
/// its other ways of failing -- an upstream that doesn't resolve, say --
|
||||||
|
/// from being reported as unrelated history: by then the merge has just
|
||||||
|
/// resolved both revisions itself.
|
||||||
|
fn unrelated_to_upstream(project: &Path) -> bool {
|
||||||
|
git(project, &["merge-base", "HEAD", "@{u}"]).is_err()
|
||||||
|
}
|
||||||
|
|
||||||
/// The phone-sized version of a failed remote command: the first line git
|
/// The phone-sized version of a failed remote command: the first line git
|
||||||
/// printed, plus what this process can see of the ssh agent when what
|
/// printed, plus what this process can see of the ssh agent when what
|
||||||
/// failed was authentication.
|
/// failed was authentication.
|
||||||
@@ -571,7 +629,7 @@ mod tests {
|
|||||||
run(&origin, &["git", "commit", "-qam", "two"]);
|
run(&origin, &["git", "commit", "-qam", "two"]);
|
||||||
|
|
||||||
fetch(&clone, true).expect("fetch with ipv4 forced");
|
fetch(&clone, true).expect("fetch with ipv4 forced");
|
||||||
pull(&clone, true).expect("pull with ipv4 forced");
|
pull(&clone, true, false).expect("pull with ipv4 forced");
|
||||||
assert!(!has_new_commits(&clone, true).expect("check"));
|
assert!(!has_new_commits(&clone, true).expect("check"));
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -622,7 +680,7 @@ mod tests {
|
|||||||
// Pull is what actually takes them.
|
// Pull is what actually takes them.
|
||||||
fetch(&clone, false).expect("fetch");
|
fetch(&clone, false).expect("fetch");
|
||||||
assert_eq!(behind(&clone), 1);
|
assert_eq!(behind(&clone), 1);
|
||||||
pull(&clone, false).expect("pull");
|
pull(&clone, false, false).expect("pull");
|
||||||
assert!(!has_new_commits(&clone, false).expect("check"));
|
assert!(!has_new_commits(&clone, false).expect("check"));
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -654,7 +712,7 @@ mod tests {
|
|||||||
"fetched but not merged is still something to pull",
|
"fetched but not merged is still something to pull",
|
||||||
);
|
);
|
||||||
|
|
||||||
pull(&clone, false).expect("pull");
|
pull(&clone, false, false).expect("pull");
|
||||||
run(
|
run(
|
||||||
&clone,
|
&clone,
|
||||||
&["git", "commit", "-q", "--allow-empty", "-m", "local"],
|
&["git", "commit", "-q", "--allow-empty", "-m", "local"],
|
||||||
@@ -759,7 +817,7 @@ mod tests {
|
|||||||
run(&origin, &["git", "commit", "-qam", "two"]);
|
run(&origin, &["git", "commit", "-qam", "two"]);
|
||||||
fetch(&clone, false).expect("fetch");
|
fetch(&clone, false).expect("fetch");
|
||||||
|
|
||||||
pull(&clone, false).expect("pull");
|
pull(&clone, false, false).expect("pull");
|
||||||
assert_eq!(behind(&clone), 0);
|
assert_eq!(behind(&clone), 0);
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
std::fs::read_to_string(clone.join("file")).expect("read"),
|
std::fs::read_to_string(clone.join("file")).expect("read"),
|
||||||
@@ -780,8 +838,12 @@ mod tests {
|
|||||||
std::fs::write(clone.join("file"), "local edit").expect("write");
|
std::fs::write(clone.join("file"), "local edit").expect("write");
|
||||||
assert!(status(&clone).expect("status").dirty);
|
assert!(status(&clone).expect("status").dirty);
|
||||||
|
|
||||||
let err = pull(&clone, false).expect_err("should refuse");
|
let err = pull(&clone, false, false).expect_err("should refuse");
|
||||||
assert!(err.contains("uncommitted changes"), "{err}");
|
assert!(
|
||||||
|
err.message.contains("uncommitted changes"),
|
||||||
|
"{}",
|
||||||
|
err.message
|
||||||
|
);
|
||||||
// Left exactly as it was.
|
// Left exactly as it was.
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
std::fs::read_to_string(clone.join("file")).expect("read"),
|
std::fs::read_to_string(clone.join("file")).expect("read"),
|
||||||
@@ -789,6 +851,63 @@ mod tests {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// The one failure the phone may override, so it has to be
|
||||||
|
/// distinguishable from every other reason a pull stops -- and the
|
||||||
|
/// override has to actually take the remote's history rather than
|
||||||
|
/// somehow merging the two.
|
||||||
|
///
|
||||||
|
/// The second half is the case the button must *not* appear on: a
|
||||||
|
/// history that has merely diverged still shares a commit, so there is
|
||||||
|
/// something better than throwing it away and this must not offer to.
|
||||||
|
#[test]
|
||||||
|
fn only_an_unrelated_history_offers_the_override() {
|
||||||
|
let dir = tempfile::tempdir().expect("tempdir");
|
||||||
|
let (origin, clone) = origin_and_clone(dir.path());
|
||||||
|
// A local history sharing no commit with the remote's, which is
|
||||||
|
// what a checkout re-created from somewhere else looks like.
|
||||||
|
run(&clone, &["git", "checkout", "-q", "--orphan", "rebuilt"]);
|
||||||
|
std::fs::write(clone.join("file"), "local").expect("write");
|
||||||
|
run(&clone, &["git", "add", "."]);
|
||||||
|
run(&clone, &["git", "commit", "-qm", "unrelated"]);
|
||||||
|
run(&clone, &["git", "branch", "-qM", "main"]);
|
||||||
|
run(
|
||||||
|
&clone,
|
||||||
|
&[
|
||||||
|
"git",
|
||||||
|
"branch",
|
||||||
|
"-q",
|
||||||
|
"--set-upstream-to=origin/main",
|
||||||
|
"main",
|
||||||
|
],
|
||||||
|
);
|
||||||
|
|
||||||
|
std::fs::write(origin.join("file"), "two").expect("write");
|
||||||
|
run(&origin, &["git", "commit", "-qam", "two"]);
|
||||||
|
fetch(&clone, false).expect("fetch");
|
||||||
|
|
||||||
|
let err = pull(&clone, false, false).expect_err("no fast-forward exists");
|
||||||
|
assert!(err.unrelated_histories, "{}", err.message);
|
||||||
|
|
||||||
|
pull(&clone, false, true).expect("forced pull");
|
||||||
|
assert_eq!(behind(&clone), 0);
|
||||||
|
assert_eq!(
|
||||||
|
std::fs::read_to_string(clone.join("file")).expect("read"),
|
||||||
|
"two"
|
||||||
|
);
|
||||||
|
|
||||||
|
// Now the two share everything up to here and each add a commit,
|
||||||
|
// which is a pull that cannot fast-forward for an entirely
|
||||||
|
// ordinary reason.
|
||||||
|
std::fs::write(clone.join("file"), "mine").expect("write");
|
||||||
|
run(&clone, &["git", "commit", "-qam", "mine"]);
|
||||||
|
std::fs::write(origin.join("file"), "theirs").expect("write");
|
||||||
|
run(&origin, &["git", "commit", "-qam", "theirs"]);
|
||||||
|
fetch(&clone, false).expect("fetch");
|
||||||
|
|
||||||
|
let err = pull(&clone, false, false).expect_err("diverged");
|
||||||
|
assert!(!err.unrelated_histories, "{}", err.message);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn a_branch_tracking_nothing_has_nothing_to_pull() {
|
fn a_branch_tracking_nothing_has_nothing_to_pull() {
|
||||||
let dir = tempfile::tempdir().expect("tempdir");
|
let dir = tempfile::tempdir().expect("tempdir");
|
||||||
@@ -799,8 +918,9 @@ mod tests {
|
|||||||
assert_eq!(status.branch, "detached-work");
|
assert_eq!(status.branch, "detached-work");
|
||||||
assert_eq!(status.upstream, None);
|
assert_eq!(status.upstream, None);
|
||||||
assert!(
|
assert!(
|
||||||
pull(&clone, false)
|
pull(&clone, false, false)
|
||||||
.expect_err("no upstream")
|
.expect_err("no upstream")
|
||||||
|
.message
|
||||||
.contains("no upstream")
|
.contains("no upstream")
|
||||||
);
|
);
|
||||||
// Nothing to compare against, so the list is told plainly rather
|
// Nothing to compare against, so the list is told plainly rather
|
||||||
|
|||||||
@@ -12,6 +12,7 @@
|
|||||||
//! PUT /roots {roots} set the directories /suggestions scans
|
//! PUT /roots {roots} set the directories /suggestions scans
|
||||||
//! GET /apps/{key}/apk[?variant=] the payload itself (ranged)
|
//! GET /apps/{key}/apk[?variant=] the payload itself (ranged)
|
||||||
//! POST /apps/{key}/pull fetch, fast-forward, and build
|
//! POST /apps/{key}/pull fetch, fast-forward, and build
|
||||||
|
//! ?force=true resets onto the upstream
|
||||||
//! POST /apps/{key}/prepare run the on-demand build step, if any
|
//! POST /apps/{key}/prepare run the on-demand build step, if any
|
||||||
//! POST /apps/{key}/build run it whether or not it looks stale
|
//! POST /apps/{key}/build run it whether or not it looks stale
|
||||||
//! POST /apps/{key}/approve accept the build step it asks for
|
//! POST /apps/{key}/approve accept the build step it asks for
|
||||||
@@ -1024,6 +1025,19 @@ struct PurgeQuery {
|
|||||||
config: bool,
|
config: bool,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Whether a pull may throw this checkout's own history away.
|
||||||
|
///
|
||||||
|
/// Off unless the phone says otherwise, and it only says so after being
|
||||||
|
/// told the checkout and its upstream are unrelated -- there is no
|
||||||
|
/// fast-forward for that ever, so the alternative to this is a card that
|
||||||
|
/// can never be pulled again. Destructive, and confirmed on the phone
|
||||||
|
/// against the branch it names before it is ever sent.
|
||||||
|
#[derive(Deserialize)]
|
||||||
|
struct PullQuery {
|
||||||
|
#[serde(default)]
|
||||||
|
force: bool,
|
||||||
|
}
|
||||||
|
|
||||||
/// Which build a download wants. Absent means the newest, which is what
|
/// Which build a download wants. Absent means the newest, which is what
|
||||||
/// every phone gets until it says otherwise.
|
/// every phone gets until it says otherwise.
|
||||||
#[derive(Deserialize)]
|
#[derive(Deserialize)]
|
||||||
@@ -1180,6 +1194,7 @@ async fn build_now(
|
|||||||
async fn build_pull(
|
async fn build_pull(
|
||||||
State(state): State<Arc<AppState>>,
|
State(state): State<Arc<AppState>>,
|
||||||
key: UrlPath<String>,
|
key: UrlPath<String>,
|
||||||
|
Query(query): Query<PullQuery>,
|
||||||
) -> Result<Json<BuildStatus>, ApiError> {
|
) -> Result<Json<BuildStatus>, ApiError> {
|
||||||
let entry = lookup(&state, Some(key))?;
|
let entry = lookup(&state, Some(key))?;
|
||||||
let build = entry
|
let build = entry
|
||||||
@@ -1191,6 +1206,7 @@ async fn build_pull(
|
|||||||
// here is the answer for the commit being replaced.
|
// here is the answer for the commit being replaced.
|
||||||
let gate = Arc::clone(&entry);
|
let gate = Arc::clone(&entry);
|
||||||
build.pull_and_build(
|
build.pull_and_build(
|
||||||
|
query.force,
|
||||||
move || gate.pending_declaration().is_none(),
|
move || gate.pending_declaration().is_none(),
|
||||||
records_builds(&state, &entry.key),
|
records_builds(&state, &entry.key),
|
||||||
);
|
);
|
||||||
|
|||||||
Reference in new issue
Block a user