feat(version upgrade): Add diff & confirmation for pkgctl version upgrade

changed the description

added 1 commit

383e5e17 - fix: Remove mistakenly added line break

added 1 commit

fbd60711 - feat(version upgrade): Add diff & confirmation for pkgctl version upgrade

Here's a PKGBUILD to build a devtools package including those changes for testing purposes:

# Maintainer: Levente Polyak <anthraxx[at]archlinux[dot]org>
# Maintainer: Christian Heusel <gromit@archlinux.org>
# Contributor: Pierre Schmitz <pierre@archlinux.de>

pkgname=devtools
epoch=1
pkgver=1.2.0
pkgrel=1
pkgdesc='Tools for Arch Linux package maintainers'
arch=('any')
license=('GPL-3.0-or-later')
url='https://gitlab.archlinux.org/antiz/devtools'
depends=(
  arch-install-scripts
  awk
  bash
  binutils
  coreutils
  curl
  diffutils
  expac
  fakeroot
  findutils
  grep
  jq
  openssh
  parallel
  rsync
  sed
  util-linux

  breezy
  git
  mercurial
  subversion
)
makedepends=(
  asciidoctor
  shellcheck
)
checkdepends=(
  bats
)
optdepends=(
  'btrfs-progs: btrfs support'
  'bat: pretty printing for pkgctl search'
  'nvchecker: pkgctl version subcommand'
)
replaces=(devtools-git-poc)
source=("git+${url}.git#branch=version_upgrade_confirm")
validpgpkeys=(
  'E240B57E2C4630BA768E2F26FC1B547C8D8172C8' # Levente Polyak <anthraxx@archlinux.org>
  'F00B96D15228013FFC9C9D0393B11DAA4C197E3D' # Christian Heusel (gromit packager key) <gromit@archlinux.org>
)
sha256sums=('SKIP')
b2sums=('SKIP')

build() {
  cd ${pkgname}
  make BUILDTOOLVER="${epoch}:${pkgver}-${pkgrel}-${arch}" PREFIX=/usr
}

check() {
  cd ${pkgname}
  make PREFIX=/usr test
}

package() {
  cd ${pkgname}
  make PREFIX=/usr DESTDIR="${pkgdir}" install
}

# vim: ts=2 sw=2 et:

mentioned in issue #239

What if the user already has some changes in the PKGBUILD before running pkgctl version upgrade?

Well, if such changes are untracked, they would be discarded indeed. I understand that might not be ideal... although the git diff beforehand shows exactly what would be reversed :P

I'll try to come up with a way to prevent that and only revert the parts modified by pkgctl version upgrade instead. I tried to play a bit with git stash but couldn't find a way to achieve that for now. Hopefully I can find a simple and elegant solution.

although the git diff beforehand shows exactly what would be reversed

Well, it is not clear from the prompt that "n" means "revert changes", one might expect that the working directory would be left dirty

Ah, fair enough! I'll make the confirmation message clearer on that front, thanks.

I added some clarifications about reverting changes in the confirmation message in !274 (eeca8d52)
Hopefully this is clear enough

I think the magic would be around not reverting changes, but instead have a confirmation to apply the new version prior to modifying the PKGBUILD and updating the checksums. That would match a much more natural expectations, not just around confirming to apply an upgrade change, but also in terms of implication on a dirty working tree.

Fair enough, that indeed sounds like a saner default behavior!

@lahwaacz @anthraxx Does it still makes sense for pkgctl version upgrade to show the actual diff of the changes it'll apply before asking for a confirmation? Or @anthraxx's proposition is just to implement a confirmation message before applying changes as it's done today?

If showing diff is still of interest, I thought of the following behavior:

pkgctl version upgrade would copy the PKGBUILD into a separate and temporary file (e.g. PKGBUILD_version_upgrade.tmp) and would make changes into that file (instead of targeting the actual PKGBUILD). Then the diff will be shown to the user with e.g. diff --unified --color PKGBUILD PKGBUILD_version_upgrade.tmp and a confirmation message will be prompted to user.

Then:

If the user gives its confirmation to proceed, then the patch is applied to the actual PKGBUILD with e.g. diff --unified --color PKGBUILD PKGBUILD_version_upgrade.tmp | git apply (the diff output can be stored in a variable or a patch file the first time it is ran if running diff twice is a concern).
If not, the patch isn't applied.

In any case, the PKGBUILD_version_upgrade.tmp file is then deleted.

Users can pass the --noconfirm flag to avoid being prompted for confirmation.

This behavior allows to avoid making changes to the PKGBUILD itself before the user gave its confirmation (and thus avoid the need to revert changes if the user doesn't give its confirmation, so we are not taking the risk to revert other untracked and unrelated changes).

How does that sounds?

For what it's worth, that would also prevent to end up with an eventual "half baked" change in the PKGBUILD (e.g. when the $pkgver is updated but the checksum for the source cannot be updated for whatever reason, like non existing upstream source or something), making the process more "atomic".

changed this line in version 6 of the diff

@anthraxx @lahwaacz The behavior described above has been applied. It was trickier than I anticipated but covers both your concerns :)
Hopefully the implement isn't too sketchy 🫣

To sum up, changes are now applied to a fresh & temporary copy of the PKGBUILD file, and later only applied to the actual PKGBUILD if the user gave the confirmation to do so. That means eventual changes made prior to running pkgctl version upgrade cannot be lost or overwritten as there's no "reverting changes" mechanism anymore. Only a diff between the PKGBUILD state right before and after pkgctl version upgrade that user has to acknowledge and confirm to actually apply.

As a side benefit, this also makes the whole process more "atomic" as the changes are only applied once the whole version upgrade process is successfully over (+ acknowledged & confirmed by the user) on the temporary file, preventing eventual undesired "half-baked" changes that can currently happen (e.g. an updated $pkgver but no updated checksums because the upstream source doesn't exists or you lost your internet connection during source downloading or whatever else...).

added 1 commit

eeca8d52 - Make the confirmation clearer about reverting changes

Compare with previous version

changed the description

added 1 commit

22a393b7 - feat(version upgrade): Add diff & confirmation for pkgctl version upgrade

Compare with previous version

added scopefeature label

added 1 commit

eb47605f - Work on a temporary file instead of the actual PKGBUILD

Compare with previous version

added 1 commit

1d888a7b - Source temporary PKGBUILD

Compare with previous version

added 1 commit

edf6d3b0 - Remove temporary file if git apply fails

Compare with previous version

added 1 commit

469f3036 - Print an error message on git apply failure

Compare with previous version

added 1 commit

9c607c80 - Fix temporary file name

Compare with previous version

marked this merge request as draft

added 1 commit

378e24d7 - Allow targeting another file than the PKGBUILD for 'pkgbuild_update_checksums'

Compare with previous version

added 1 commit

be5d60d5 - Apply changes right away if user used the --noconfirm flag

Compare with previous version

added 1 commit

51e966ce - Make it work when working on multiple packages

Compare with previous version

 			"${#failure[@]}"
 	fi
 	# ask confirmation about changes made to PKGBUILD
 	if (( noconfirm == 0 )); then
 		echo
 		git diff PKGBUILD
 		read -rep $'\nConfirm changes? [Y/n] ' answer
 		case $answer in
 			"Y"|"y"|"")
 				true
 			;;
 			*)
 				printf "\nReverting changes...\n"
 				git checkout PKGBUILD

Admin message

feat(version upgrade): Add diff & confirmation for pkgctl version upgrade

Merge request reports

Activity