When fetchpatchsorts hunks, it can move hunks that change a file before the hunk that creates the file. This can cause the patch to fail to apply when patch attempts to modify a file that doesn't exist yet.
Gets written to the Nix store like this; note the hunks which modify m4/fp_ld_supports_response_files.m4 have been moved before the hunk that creates it, an issue not present with the original commit ordering.
This reordering is meant to make fetched patches more robust to changes in the source URL, but in some cases it can break the patches and make them unusable. There is also no mechanism in nixpkgs for disabling this behavior. Introducing an option to disable this sorting would be very useful.
Artturin
commented
1 year ago
Describe the bug
When
fetchpatchsorts hunks, it can move hunks that change a file before the hunk that creates the file. This can cause the patch to fail to apply whenpatchattempts to modify a file that doesn't exist yet.For example, this patch URL:
https://gitlab.haskell.org/ghc/ghc/-/merge_requests/9897.patch
Gets written to the Nix store like this; note the hunks which modify
m4/fp_ld_supports_response_files.m4have been moved before the hunk that creates it, an issue not present with the original commit ordering.GHC Merge Request 9897
```patch --- a/compiler/GHC/Settings.hs +++ b/compiler/GHC/Settings.hs @@ -19,6 +19,7 @@ , sGlobalPackageDatabasePath , sLdSupportsCompactUnwind , sLdSupportsFilelist + , sLdSupportsResponseFiles , sLdIsGnuLd , sGccSupportsNoPie , sUseInplaceMinGW @@ -87,6 +88,7 @@ data ToolSettings = ToolSettings { toolSettings_ldSupportsCompactUnwind :: Bool , toolSettings_ldSupportsFilelist :: Bool + , toolSettings_ldSupportsResponseFiles :: Bool , toolSettings_ldIsGnuLd :: Bool , toolSettings_ccSupportsNoPie :: Bool , toolSettings_useInplaceMinGW :: Bool @@ -189,6 +191,8 @@ sLdSupportsCompactUnwind = toolSettings_ldSupportsCompactUnwind . sToolSettings sLdSupportsFilelist :: Settings -> Bool sLdSupportsFilelist = toolSettings_ldSupportsFilelist . sToolSettings +sLdSupportsResponseFiles :: Settings -> Bool +sLdSupportsResponseFiles = toolSettings_ldSupportsResponseFiles . sToolSettings sLdIsGnuLd :: Settings -> Bool sLdIsGnuLd = toolSettings_ldIsGnuLd . sToolSettings sGccSupportsNoPie :: Settings -> Bool --- a/compiler/GHC/Settings/IO.hs +++ b/compiler/GHC/Settings/IO.hs @@ -95,6 +95,7 @@ cxx_args = words cxx_args_str ldSupportsCompactUnwind <- getBooleanSetting "ld supports compact unwind" ldSupportsFilelist <- getBooleanSetting "ld supports filelist" + ldSupportsResponseFiles <- getBooleanSetting "ld supports response files" ldIsGnuLd <- getBooleanSetting "ld is GNU ld" arSupportsDashL <- getBooleanSetting "ar supports -L" @@ -163,6 +164,7 @@ , sToolSettings = ToolSettings { toolSettings_ldSupportsCompactUnwind = ldSupportsCompactUnwind , toolSettings_ldSupportsFilelist = ldSupportsFilelist + , toolSettings_ldSupportsResponseFiles = ldSupportsResponseFiles , toolSettings_ldIsGnuLd = ldIsGnuLd , toolSettings_ccSupportsNoPie = gccSupportsNoPie , toolSettings_useInplaceMinGW = useInplaceMinGW --- a/compiler/GHC/SysTools/Tasks.hs +++ b/compiler/GHC/SysTools/Tasks.hs @@ -29,7 +29,6 @@ import GHC.Utils.Misc import GHC.Utils.Logger import GHC.Utils.TmpFs -import GHC.Utils.Constants (isWindowsHost) import GHC.Utils.Panic import Data.List (tails, isPrefixOf) @@ -350,9 +349,7 @@ , "does not support object merging." ] optl_args = map Option (getOpts dflags opt_lm) args2 = args0 ++ args ++ optl_args - -- N.B. Darwin's ld64 doesn't support response files. Consequently we only - -- use them on Windows where they are truly necessary. - if isWindowsHost + if toolSettings_ldSupportsResponseFiles (toolSettings dflags) then do mb_env <- getGccEnv args2 runSomethingResponseFile logger tmpfs dflags id "Merge objects" p args2 mb_env --- a/configure.ac +++ b/configure.ac @@ -663,6 +663,8 @@ FP_LD_NO_FIXUP_CHAINS([target], [CONF_GCC_LINKER_OPTS_STAGE1]) FP_LD_NO_FIXUP_CHAINS([target], [CONF_GCC_LINKER_OPTS_STAGE2]) +FP_LD_SUPPORTS_RESPONSE_FILES() + GHC_LLVM_TARGET_SET_VAR # we intend to pass trough --targets to llvm as is. LLVMTarget_CPP=` echo "$LlvmTarget"` --- a/configure.ac +++ b/configure.ac @@ -663,7 +663,7 @@ FP_LD_NO_FIXUP_CHAINS([target], [CONF_GCC_LINKER_OPTS_STAGE1]) FP_LD_NO_FIXUP_CHAINS([target], [CONF_GCC_LINKER_OPTS_STAGE2]) -FP_LD_SUPPORTS_RESPONSE_FILES() +FP_LD_SUPPORTS_RESPONSE_FILES GHC_LLVM_TARGET_SET_VAR # we intend to pass trough --targets to llvm as is. --- a/distrib/configure.ac.in +++ b/distrib/configure.ac.in @@ -180,6 +180,8 @@ FP_LD_NO_FIXUP_CHAINS([target], [CONF_GCC_LINKER_OPTS_STAGE1]) FP_LD_NO_FIXUP_CHAINS([target], [CONF_GCC_LINKER_OPTS_STAGE2]) +FP_LD_SUPPORTS_RESPONSE_FILES + AC_SUBST(CONF_CC_OPTS_STAGE0) AC_SUBST(CONF_CC_OPTS_STAGE1) AC_SUBST(CONF_CC_OPTS_STAGE2) --- a/hadrian/bindist/Makefile +++ b/hadrian/bindist/Makefile @@ -92,6 +92,7 @@ @echo ',("ld flags", "$(SettingsLdFlags)")' >> $@ @echo ',("ld supports compact unwind", "$(LdHasNoCompactUnwind)")' >> $@ @echo ',("ld supports filelist", "$(LdHasFilelist)")' >> $@ + @echo ',("ld supports response files", "$(LdSupportsResponseFiles)")' >> $@ @echo ',("ld is GNU ld", "$(LdIsGNULd)")' >> $@ @echo ',("Merge objects command", "$(SettingsMergeObjectsCommand)")' >> $@ @echo ',("Merge objects flags", "$(SettingsMergeObjectsFlags)")' >> $@ --- a/hadrian/bindist/config.mk.in +++ b/hadrian/bindist/config.mk.in @@ -236,6 +236,7 @@ GccExtraViaCOpts = @GccExtraViaCOpts@ LdHasFilelist = @LdHasFilelist@ +LdSupportsResponseFiles = @LdSupportsResponseFiles@ LdHasBuildId = @LdHasBuildId@ LdHasFilelist = @LdHasFilelist@ LdIsGNULd = @LdIsGNULd@ --- a/hadrian/cfg/system.config.in +++ b/hadrian/cfg/system.config.in @@ -140,6 +140,7 @@ gcc-extra-via-c-opts = @GccExtraViaCOpts@ ld-has-no-compact-unwind = @LdHasNoCompactUnwind@ ld-has-filelist = @LdHasFilelist@ +ld-supports-response-files = @LdSupportsResponseFiles@ ld-is-gnu-ld = @LdIsGNULd@ ar-args = @ArArgs@ --- a/hadrian/src/Rules/Generate.hs +++ b/hadrian/src/Rules/Generate.hs @@ -427,6 +427,7 @@ , ("ld flags", expr $ settingsFileSetting SettingsFileSetting_LdFlags) , ("ld supports compact unwind", expr $ lookupSystemConfig "ld-has-no-compact-unwind") , ("ld supports filelist", expr $ lookupSystemConfig "ld-has-filelist") + , ("ld supports response files", expr $ lookupSystemConfig "ld-supports-response-files") , ("ld is GNU ld", expr $ lookupSystemConfig "ld-is-gnu-ld") , ("Merge objects command", expr $ settingsFileSetting SettingsFileSetting_MergeObjectsCommand) , ("Merge objects flags", expr $ settingsFileSetting SettingsFileSetting_MergeObjectsFlags) --- a/m4/fp_ld_supports_response_files.m4 +++ b/m4/fp_ld_supports_response_files.m4 @@ -1,6 +1,6 @@ # FP_LD_SUPPORTS_RESPONSE_FILES # -------------------- -# See if whether we are using a version of ld64 on darwin platforms which +# See if whether we are using a version of ld which # supports response files. AC_DEFUN([FP_LD_SUPPORTS_RESPONSE_FILES], [ AC_MSG_CHECKING([whether $LD supports response files]) --- a/m4/fp_ld_supports_response_files.m4 +++ b/m4/fp_ld_supports_response_files.m4 @@ -6,7 +6,7 @@ AC_MSG_CHECKING([whether $LD supports response files]) echo 'int main(void) {return 0;}' > conftest.c $CC -c -o conftest.o conftest.c > /dev/null 2>&1 - if $LD @<(printf '%q\n' -o conftest conftest.o) > /dev/null 2>&1 + if $LD @<(printf '%q\n' -shared -o conftest conftest.o) > /dev/null 2>&1 || $LD @<(printf '%q\n' -dylib -o conftest conftest.o) > /dev/null 2>&1 then LdSupportsResponseFiles=YES AC_MSG_RESULT([yes]) --- a/m4/fp_ld_supports_response_files.m4 +++ b/m4/fp_ld_supports_response_files.m4 @@ -1,12 +1,11 @@ # FP_LD_SUPPORTS_RESPONSE_FILES # -------------------- -# See if whether we are using a version of ld which -# supports response files. +# See if whether we are using a version of ld which supports response files. AC_DEFUN([FP_LD_SUPPORTS_RESPONSE_FILES], [ AC_MSG_CHECKING([whether $LD supports response files]) echo 'int main(void) {return 0;}' > conftest.c - $CC -c -o conftest.o conftest.c > /dev/null 2>&1 - if $LD @<(printf '%q\n' -shared -o conftest conftest.o) > /dev/null 2>&1 || $LD @<(printf '%q\n' -dylib -o conftest conftest.o) > /dev/null 2>&1 + "$CC" -c -o conftest.o conftest.c > /dev/null 2>&1 + if "$LD" @<(printf '%q\n' -shared -o conftest conftest.o) > /dev/null 2>&1 || "$LD" @<(printf '%q\n' -dylib -o conftest conftest.o) > /dev/null 2>&1 then LdSupportsResponseFiles=YES AC_MSG_RESULT([yes]) --- a/m4/fp_ld_supports_response_files.m4 +++ b/m4/fp_ld_supports_response_files.m4 @@ -5,7 +5,8 @@ AC_MSG_CHECKING([whether $LD supports response files]) echo 'int main(void) {return 0;}' > conftest.c "$CC" -c -o conftest.o conftest.c > /dev/null 2>&1 - if "$LD" @<(printf '%q\n' -shared -o conftest conftest.o) > /dev/null 2>&1 || "$LD" @<(printf '%q\n' -dylib -o conftest conftest.o) > /dev/null 2>&1 + printf '%q\n' -shared -o conftest conftest.o > args.txt + if "$LD" -shared @args.txt > /dev/null 2>&1 || "$LD" -dylib @args.txt > /dev/null 2>&1 then LdSupportsResponseFiles=YES AC_MSG_RESULT([yes]) --- a/m4/fp_ld_supports_response_files.m4 +++ b/m4/fp_ld_supports_response_files.m4 @@ -5,7 +5,7 @@ AC_MSG_CHECKING([whether $LD supports response files]) echo 'int main(void) {return 0;}' > conftest.c "$CC" -c -o conftest.o conftest.c > /dev/null 2>&1 - printf '%q\n' -shared -o conftest conftest.o > args.txt + printf '%q\n' -o conftest conftest.o > args.txt if "$LD" -shared @args.txt > /dev/null 2>&1 || "$LD" -dylib @args.txt > /dev/null 2>&1 then LdSupportsResponseFiles=YES @@ -14,6 +14,6 @@ LdSupportsResponseFiles=NO AC_MSG_RESULT([no]) fi - rm -f conftest.c conftest + rm -f conftest.c conftest args.txt AC_SUBST(LdSupportsResponseFiles) ]) --- /dev/null +++ b/m4/fp_ld_supports_response_files.m4 @@ -0,0 +1,19 @@ +# FP_LD_SUPPORTS_RESPONSE_FILES +# -------------------- +# See if whether we are using a version of ld64 on darwin platforms which +# supports response files. +AC_DEFUN([FP_LD_SUPPORTS_RESPONSE_FILES], [ + AC_MSG_CHECKING([whether $LD supports response files]) + echo 'int main(void) {return 0;}' > conftest.c + $CC -c -o conftest.o conftest.c > /dev/null 2>&1 + if $LD @<(printf '%q\n' -o conftest conftest.o) > /dev/null 2>&1 + then + LdSupportsResponseFiles=YES + AC_MSG_RESULT([yes]) + else + LdSupportsResponseFiles=NO + AC_MSG_RESULT([no]) + fi + rm -f conftest.c conftest + AC_SUBST(LdSupportsResponseFiles) +]) ```Additional Context
This reordering is meant to make fetched patches more robust to changes in the source URL, but in some cases it can break the patches and make them unusable. There is also no mechanism in
nixpkgsfor disabling this behavior. Introducing an option to disable this sorting would be very useful.Related Issues