search

LongDirtyAnimAlf / Reiniero-fpcup

fpcup (and fpclazup) are installers for FPC (and Lazarus).
52 stars 10 forks source link

Regression: "patchcmd" option no longer functional #48

Closed martok closed 2 years ago

martok commented 2 years ago

Hi!

A bit late to notice that, but in d148fbc5095107 (LongDirtyAnimAlf/fpcupdeluxe@d35d27ba), the command line option --patchcmd got disabled: https://github.com/LongDirtyAnimAlf/Reiniero-fpcup/blob/002c40ca14f7fcc552ad9e30de6cf7515fe84ec9/checkoptions.pas#L233

So I just end up with

Error: wrong command line options given: patchcmd=git apply -p0

Press enter to see a list of all available command line options.

Was there a particular reason for that?

LongDirtyAnimAlf commented 2 years ago

Will try to revert this change.

LongDirtyAnimAlf commented 2 years ago

Please test the latest release. Thanks.

martok commented 2 years ago

At some point it stopped accepting a commandline (due to setting .Executable ), but that's fine using a wrapper script:

... --patchcmd="patchwrap.cmd" --fpcpatch="patches\patch1.diff,patchn.patch" ...
:: patchwrap.cmd
@echo off
git apply --stat --apply -v --recount -p0 "%1"
exit /b %errorlevel%

Issue fixed, thank you!

LongDirtyAnimAlf commented 2 years ago

Thanks for the feedback. Its hard for the current code to handle a patch command like yours. However, it would be good if it could. In the current case, that would mean parsing of the custom command and feeding that into the processor. Are you in favor of that ? Or is your current situation acceptable for you ? Thanks

martok commented 2 years ago

This patch command eats SVN and Git patches, it's what I use for stuff plucked from the bugtracker over the years. Technically I could (should) just track a proper fork now that the Git switch is done, but until I get around to setting that up, patch files it is. So, mostly a "me-problem" ;-)

IIRC TProcess can do the splitting on its own by setting CommandLine(?) instead of Executable, but that has some quirks and it probably makes no sense to add that complexity just for the limited use. Nobody else seems to have noticed that it was gone, so the user base seems to be very limited. As long as calling a script works, I see no major problem.