failed installation of VCS packages, that patch sources inside prepare()

[fftmp]

pikaur -Vq
Pikaur v1.12
Pacman v6.0.1 - libalpm v13.0.1 - pyalpm v0.10.6

Description:

Can't install some packages, for example libopenmpt-doc-svn. Looks like pikaur runs makepkg twice for VCS packages (i.e. whose names ends with '-git', '-svn', etc) - firstly to get version and secondary to build. So after fix #661 prepare() runs two times also. If prepare() is not reenterable, build failed (and it must slow build for long-term prepare() functions). Is it bug or expected behaviour?

If this is expected behaviour, how can package maintainers deal with that? Typically applying patches is inside prepare() (as for libopenmpt-doc-svn), so second patch call fails. I can see next approaches to make prepare() reenterable in PKGBUILD (i.e. without changes in pikaur):

1. patch sources inside build()
2. cleanup source tree in prepare() before applying patches
3. mask patch exit codes, i.e. patch -p1 -i <some.patch> || true

Seems, there are very little amount of such packages, I meet this for apr-2-svn (which fixed using method 2).

Attached log:

$ LANG=en pikaur -S libopenmpt-doc-svn --verbose --pikaur-debug 
:: debug: main: Pikaur operation found for sys.argv=['/usr/bin/pikaur', '-S', 'libopenmpt-doc-svn', '--verbose', '--pikaur-debug']: cli_install_packages
:: debug: install_info_fetcher: Gonna fetch install info for:
    install_package_names=['libopenmpt-doc-svn']
    not_found_repo_pkgs_names=[]
    pkgbuilds_packagelists={}
    manually_excluded_packages_names=[]

:: debug: install_info_fetcher: Gonna get repo pkgs install info...
Reading repository package databases...
Reading local package database...
=> pacman --color=always --sync libopenmpt-doc-svn --print-format %r/%n
=> pacman --color=always --sync libopenmpt-doc-svn --print-format %r/%n
:: debug: install_info_fetcher: gonna get AUR pkgs install info for:
    aur_packages_versionmatchers=['libopenmpt-doc-svn']
    self.aur_updates_install_info=[]
    aur_packages_names_to_versions={'libopenmpt-doc-svn': <VersionMatcher libopenmpt-doc-svn['cmp_default']None>}
=> GET https://aur.archlinux.org/rpc/?v=5&type=info&arg[]=libopenmpt-doc-svn
:: debug: install_info_fetcher: found AUR pkgs:
    aur_pkg_list=[<AURPackageInfo "libopenmpt-doc-svn" 0.7.r16685-1>]
    not_found_aur_pkgs=[]

:: debug: install_info_fetcher: got AUR pkgs install info: self.aur_updates_install_info=[<AURInstallInfo "libopenmpt-doc-svn"   -> 0.7.r16685-1>]
Resolving AUR dependencies...
=> pacman --color=always --deptest subversion doxygen flac help2man libpulse libsndfile libvorbis mpg123 portaudio sdl2 zlib

:: AUR package will be installed:
 libopenmpt-doc-svn                                         -> 0.7.r16685-1

:: debug: prompt: Gonna get input from user...
:: debug: prompt_nolock: Restoring TTY...
:: debug: prompt_nolock: Using standard input reader...
:: Proceed with installation? [Y/n] 
:: [v]iew package details   [m]anually select packages
>> y
:: debug: prompt_nolock: Reverting to prev TTY state...
:: debug: prompt: Got answer: 'y'
=> git -C /home/fft/.local/share/pikaur/aur_repos/libopenmpt-svn pull origin master
looking for conflicting AUR packages...
:: warning: Not showing diff for libopenmpt-doc-svn package (installing for the first time)
:: debug: prompt: Gonna get input from user...
:: debug: prompt_nolock: Restoring TTY...
:: debug: prompt_nolock: Using standard input reader...
Do you want to edit PKGBUILD for libopenmpt-doc-svn package? [Y/n] n
:: debug: prompt_nolock: Reverting to prev TTY state...
:: debug: prompt: Got answer: 'n'

:: debug: install_cli: Gonna build self.package_builds_by_name={'libopenmpt-doc-svn': <pikaur.build.PackageBuild object at 0x7f10a4900610>}
:: debug: install_cli: Gonna build pkg_build.package_names=['libopenmpt-doc-svn']
=> mkdir -p /home/fft/.cache/pikaur/build/libopenmpt-svn
=> cp -r /home/fft/.local/share/pikaur/aur_repos/libopenmpt-svn/PKGBUILD /home/fft/.local/share/pikaur/aur_repos/libopenmpt-svn/010-libopenmpt-fix-doc-install-dir.patch /home/fft/.local/share/pikaur/aur_repos/libopenmpt-svn/.SRCINFO /home/fft/.cache/pikaur/build/libopenmpt-svn/
:: Downloading the latest sources for a devel package libopenmpt-doc-svn...
=> makepkg --nobuild --nocheck --nodeps
=> makepkg --printsrcinfo -p PKGBUILD
=> makepkg --packagelist

:: Starting the build:
=> makepkg --force
==> Making package: libopenmpt-svn 0.7.r18013-1 (Thu Oct 20 16:49:55 2022)
==> Checking runtime dependencies...
==> Checking buildtime dependencies...
==> Retrieving sources...
  -> Updating OpenMPT svn repo...
Updating '.':
At revision 18013.
  -> Found 010-libopenmpt-fix-doc-install-dir.patch
==> Validating source files with sha256sums...
    OpenMPT ... Skipped
    010-libopenmpt-fix-doc-install-dir.patch ... Passed
==> Extracting sources...
  -> Creating working copy of  svn repo...
==> Starting prepare()...
patching file Makefile
Reversed (or previously applied) patch detected!  Skipping patch.
1 out of 1 hunk ignored -- saving rejects to file Makefile.rej
==> ERROR: A failure occurred in prepare().
    Aborting...

Command 'makepkg --force' failed to execute.
:: debug: prompt: Gonna get input from user...
:: debug: prompt_nolock: Restoring TTY...
:: debug: prompt_nolock: Using standard input reader...
:: Try recovering?
[R] retry build
[p] PGP check skip
[c] checksums skip
[f] skip 'check()' function of PKGBUILD
[i] ignore architecture
[d] delete build dir and try again
[e] edit PKGBUILD
------------------------
[s] skip building this package
[a] abort building all the packages
> a

[actionless]

i believe that these 3 fix-points to the pkgbuild need to be done disregard of pikaur behavior - otherwise the package would behave funny just when building it locally twice (first to install, second time to upgrade). depending if update is available or not it will either fail the same way or would cause merge conflict when retrieving the sources

but if you or anyone else got an ideas how to improve it on pikaur side - i'm open for the suggestions/patches

[actionless]

btw a potential fix from pikaur side could be to add the following item to the build recovery menu:

:: Try recovering?
[R] retry build
...
[f] skip 'check()' function of PKGBUILD
 >> [?] skip 'prepare()' function of PKGBUILD <<
...

[fftmp]

prepare() function is intended for patching, both logically and according to archwiki, so seems, that fix-point 1 is wrong way. fix-point 3 is both wrong and danger way, because it just mask an errors. Theoretically patch can be applied twice, what can lead to unpredictable behaviour. It seems for me, that item in recovery menu is wrong way also, because nobody understand, that problem was caused by multiple prepare() invocations. So fix-point 2 is better way for now.

About troubles related to building locally twice. For VCS packages makepkg don't redownload sources if source dir exists, so it can't be used for upgrade anyway (need to use makepkg -C or update sources manually). For rebuilding - yes, that is problem. Scenario (VCS package + patch sources + manual rebuild) looks exotic (BTW, whether this can happen with retry option in pikaur?), but this is reason to revert source tree to initial state at prepare(). Anyway, such problem is not the reasson to call prepare() twice.

As a variant: is it ok to call makepkg for the second time with --noprepare for dev packages? i.e. 1st call: prepare->pkgver 2nd call: build->check->package

BTW, ttf-iosevka-custom-git PKGBUILD looks very strange for me, because

1. Build depends on some file inside $HOME
2. It calls npm update inside build(). I'm not familiar with nodejs, but seems, that this cause update of all installed nodejs packages for current user. Very curious side-effect, if that's true.
3. This package get sources in prepare() rather than specify it in source() array. Seems, this is the reason, why pkgver() failed for this package.

[actionless]

yup 2nd seems the best of these 3. and regarding fixing it on pikaur side - i first of all wonder if any of PKGBUILD spec mention anything such sort of situation, if prepare() or other functions meant to be run multiple times

i agree that iosevka package is weird, but main point of that ticket was the spec of PKGBUILD which was describing such execution order of the sections

[fftmp]

The best thing, that I found, is post from one of developers. If I understand correctly, extracting sources and prepare()

is something that should be done exactly once in an idempotent manner before building

There is also related issue, but no helpful info there.

[actionless]

that should be done exactly once in an idempotent manner before building

since build in pikaur is implemented as class that not complicated to implement that as just a property "prepare_executed" - so it will preserve - but would be nice to have more clear grounding for that before implementing any logic