ds300 / patch-package

Fix broken node modules instantly πŸƒπŸ½β€β™€οΈπŸ’¨
MIT License
10.48k stars 295 forks source link

`patch-package --partial` returns errors for incorrect patch #485

Open karlhorky opened 1 year ago

karlhorky commented 1 year ago

First of all, thanks again for all of the hard work and maintenance that goes into patch-package. It's also an inspiration for other tools in the ecosystem, leading to the pnpm patch feature (although patch-package is still a bit ahead in terms of DX).

I noticed a new error message which I thought showed an exciting new feature called --partial:

β›” ERROR

Failed to apply patch file next+13.4.18.patch.

If this patch file is no longer useful, delete it and run

  patch-package

To partially apply the patch (if possible) and output a log of errors to fix, run

  patch-package --partial

After which you should make any required changes inside node_modules/next, and finally run

  patch-package next

to update the patch file.

So I decided to give it a try, and ran yarn patch-package --partial:

$  yarn patch-package --partial
patch-package 8.0.0
Applying patches...
Saving errors to ./patch-package-errors.log

...as expected, I received errors - I expected these errors to match the updated next package and the patch which was failing, so that I could manually edit the offending files in node_modules/next.

However, the error log file contained errors about a completely different patch - one that actually does not have troubles applying, if I revert the version of the offending next package.

./patch-package-errors.log

Cannot apply hunk 0 for file node_modules/check-links/lib/check-link.js
```diff
@@ -3,6 +3,7 @@
 const got = require('got')
 const isRelativeUrl = require('is-relative-url')
 const parse = require('url').parse
+const userAgents = require('top-user-agents')

 const protocolWhitelist = new Set([
   'https:',

Cannot apply hunk 1 for file node_modules/check-links/lib/check-link.js

@@ -3,6 +3,7 @@
 const got = require('got')
 const isRelativeUrl = require('is-relative-url')
 const parse = require('url').parse
+const userAgents = require('top-user-agents')

 const protocolWhitelist = new Set([
   'https:',
@@ -89,4 +90,4 @@ module.exports.isValidUrl = (url, opts) => {
   }
 }

-module.exports.userAgent = 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/67.0.3396.99 Safari/537.36'
+module.exports.userAgent = userAgents[Math.floor(Math.random() * (userAgents.length - 1))]


---

One potentially related thing is that I noticed that `patch-package` started [retaining old, obsolete patch files](https://github.com/ds300/patch-package/issues/480) when upgrading dependencies, and this was also the case for the `next` package - I had two identical patches from when I ran `yarn patch-package next` the last time I upgraded versions. However, this does not appear to be related to `--partial` mode, because:

1. Removing the extra patch file did not resolve the bug reported above
2. The extra patch file was not causing any other problems
meramsey commented 1 year ago

I also seen this where if its already applied the patch it falsely says it cannot apply it vs saying its already applied. It should be able to try reversing a patch to confirm if its already applied it vs erroring out that it cannot apply.

Also i find running: npx patch-package --reverse; npx patch-package --partial between attempts ensures i don't run into those issues as im resolving and fixing patches.

I just wish the --partial had an optional flag or defaulted to making a list of all the individual files that would fail vs stopping at the first one. Would be super useful to just or also list out the broken patch files all at once so they can be run through and fixed before i retry applying to incrementally find them all which is tedious.

I had a helper script i was using before the v8 changes which while not perfect was kinda useful for seeing a list of broken patches from my old manual setup but you can get the idea from it.

/**
 * PatchMeIfYouCan.js
 *
 * Usage:
 * -------
 * Dry run mode (checks if patches can be applied without actually applying them):
 * node scripts/PatchMeIfYouCan.js --dry-run
 *
 * Real mode (applies patches):
 * node scripts/PatchMeIfYouCan.js
 *
 * Real mode with merge conflicts (applies patches and leaves conflict markers for manual resolution):
 * node scripts/PatchMeIfYouCan.js --merge
 *
 * Verbose mode (shows extra logs for debugging):
 * node scripts/PatchMeIfYouCan.js --verbose
 *
 * Description:
 * ------------
 * This script processes patch files in the 'patches' folder, applying them to the
 * corresponding target files. It supports dry run, real, and real with merge conflict modes.
 * In dry run mode, it checks if the patch can be applied without actually applying it.
 * In real mode, it applies the patches to the target files. In real mode with merge conflicts,
 * it applies the patches and leaves conflict markers for manual resolution when a conflict is detected.
 *
 * The script recursively processes all folders within the 'patches' root folder,
 * excluding any 'original_patch_filename.txt' files found within.
 */

const fs = require('fs');
const path = require('path');
const { exec } = require('child_process');

const patchesRootFolder = 'patches';

// Define the fuzz factor for patching
const fuzzFactor = 20;

const checkIfPatchAlreadyApplied = (patchFile, targetFile, callback) => {
  const reversePatchCommand = `patch --ignore-whitespace -R --fuzz=${fuzzFactor} --silent --forward --no-backup-if-mismatch "${targetFile}" < "${patchFile}"`;

  exec(reversePatchCommand, (error, stdout, stderr) => {
    if (error) {
      // The patch has NOT been applied before.
      callback(false);
    } else {
      // The patch has been applied before.
      callback(true);
    }
  });
};

const applyPatch = (patchFile, targetFile, options, callback) => {
  checkIfPatchAlreadyApplied(patchFile, targetFile, (alreadyApplied) => {
    if (alreadyApplied) {
      console.log(`βœ“ Patch ${patchFile} has already been applied.`);
      callback(null, 'Already applied', 'Already applied');
      return;
    }

    const patchCommand = `patch --ignore-whitespace --fuzz=${fuzzFactor} ${
      options.dryRun ? '--dry-run' : ''
    } --silent --forward --no-backup-if-mismatch ${
      options.merge ? '--merge' : ''
    } "${targetFile}" < "${patchFile}"`;

    exec(patchCommand, (error, stdout, stderr) => {
      // if (error) {
      //   if (options.verbose) {
      //     console.log(`Failed to apply ${patchFile}. Command used: ${patchCommand}`);
      //     console.log(`STDOUT: ${stdout}`);
      //     console.log(`STDERR: ${stderr}`);
      //   } else {
      //     //console.log(`βœ— Failed to apply ${patchFile}`);
      //   }
      // } else {
      //   //console.log(`βœ“ Successfully applied ${patchFile}`);
      // }
      callback(error, stdout, stderr);
    });
  });
};

const processPatchesFolder = (patchesFolder, options) => {
  fs.readdir(patchesFolder, (err, files) => {
    if (err) {
      console.error(`Error reading directory: ${err.message}`);
      return;
    }

    files.forEach((file) => {
      if (file === 'original_patch_filename.txt') {
        return;
      }

      const patchFile = path.join(patchesFolder, file);

      // Extract module name from folder name
      const moduleName = path.basename(patchesFolder);

      // Create the target file path based on the name of the patch file
      // Replace underscores with slashes and append it to the 'node_modules/' and module name
      const targetFileName = file.replace(/_patch$/, '').replace(/_/g, '/');
      const targetFile = path.join('node_modules', moduleName, targetFileName);

      applyPatch(patchFile, targetFile, options, (error, stdout, stderr) => {
        if (error) {
          // console.log(`${"\033[0;31mβœ—\033[0m"} Failed to apply ${patchFile} error: ${stdout.replace(/\s+$/g, '')}`);
          console.log(`${"\033[0;31mβœ—\033[0m"} Failed to apply ${patchFile}`);
        } else {
          console.log(`${"\033[0;32mβœ“\033[0m"} Successfully applied ${patchFile}`);
        }
      });
    });
  });
};

const main = () => {
  const args = process.argv.slice(2);

  const options = {
    dryRun: args.includes('--dry-run'),
    merge: args.includes('--merge'),
    // verbose: args.includes('--verbose'),
  };

  fs.readdir(patchesRootFolder, (err, folders) => {
    if (err) {
      console.error(`Error reading patches root directory: ${err.message}`);
      return;
    }

    folders.forEach((folder) => {
      const patchesFolder = path.join(patchesRootFolder, folder);
      fs.stat(patchesFolder, (err, stats) => {
        if (err) {
          console.error(`Error checking folder stats: ${err.message}`);
          return;
        }

        if (stats.isDirectory()) {
          processPatchesFolder(patchesFolder, options);
        }
      });
    });
  });
};

main();