| Summary: | oe/patch.py: PATCHTOOL = "patch" doesn't work | ||
|---|---|---|---|
| Product: | [Build System, Metadata & Runtime] OE-Core | Reporter: | Robert Yang <liezhi.yang> |
| Component: | core | Assignee: | Robert Yang <liezhi.yang> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | Low | CC: | andrei, jessica.zhang, kergoth, Martin.Jansa, meta.mr.watcher, meta.watcher, ross.burton, sgw |
| Version: | 1.6 | ||
| Target Milestone: | 1.8 | ||
| Hardware: | x86 | ||
| OS: | Multiple | ||
| Whiteboard: | |||
| OS type for building Yocto: | --- | Type of Regression: | --- |
| Verified: | Documentation change: | Don't know | |
|
Description
Robert Yang
2014-03-14 08:57:00 UTC
And we should set TMPDIR and unset it in meta/lib/oe/patch.py rather than in patch.bbclass, here is a draft patch:
diff --git a/meta/classes/patch.bbclass b/meta/classes/patch.bbclass
index 86c65b3..2ade0a8 100644
--- a/meta/classes/patch.bbclass
+++ b/meta/classes/patch.bbclass
@@ -139,13 +139,6 @@ python patch_do_patch() {
path = os.getenv('PATH')
os.putenv('PATH', d.getVar('PATH', True))
- # We must use one TMPDIR per process so that the "patch" processes
- # don't generate the same temp file name.
-
- import tempfile
- process_tmpdir = tempfile.mkdtemp()
- os.environ['TMPDIR'] = process_tmpdir
-
for patch in src_patches(d):
_, _, local, _, _, parm = bb.fetch.decodeurl(patch)
@@ -168,15 +161,11 @@ python patch_do_patch() {
try:
patchset.Import({"file":local, "strippath": parm['striplevel']}, True)
except Exception as exc:
- bb.utils.remove(process_tmpdir, True)
bb.fatal(str(exc))
try:
resolver.Resolve()
except bb.BBHandledException as e:
- bb.utils.remove(process_tmpdir, True)
bb.fatal(str(e))
-
- bb.utils.remove(process_tmpdir, True)
}
patch_do_patch[vardepsexclude] = "PATCHRESOLVE"
diff --git a/meta/lib/oe/patch.py b/meta/lib/oe/patch.py
index b085c9d..71c0589 100644
--- a/meta/lib/oe/patch.py
+++ b/meta/lib/oe/patch.py
@@ -138,6 +138,8 @@ class PatchTree(PatchSet):
self.patches.insert(i, patch)
def _applypatch(self, patch, force = False, reverse = False, run = True):
+ import tempfile
+
shellcmd = ["cat", patch['file'], "|", "patch", "-p", patch['strippath']]
if reverse:
shellcmd.append('-R')
@@ -154,7 +156,16 @@ class PatchTree(PatchSet):
return
shellcmd.pop(len(shellcmd) - 1)
- output = runcmd(["sh", "-c", " ".join(shellcmd)], self.dir)
+
+ # We must use one TMPDIR per process so that the "patch" processes
+ # don't generate the same temp file name.
+ process_tmpdir = tempfile.mkdtemp()
+ os.environ['TMPDIR'] = process_tmpdir
+ try:
+ output = runcmd(["sh", "-c", " ".join(shellcmd)], self.dir)
+ finally:
+ bb.utils.remove(process_tmpdir, True)
+ os.unsetenv('TMPDIR')
if not reverse:
self._appendPatchFile(patch['file'], patch['strippath'])
This has the same problem: if TMPDIR was already set in the environment you wipe it. I was thinking more along the lines of passing TMPDIR=/my/new/tmp/path directly to sh in the runcmd call so it's only valid in the shell. Then again is there a reason to use patch instead of quilt here? (In reply to comment #2) > This has the same problem: if TMPDIR was already set in the environment you > wipe it. > > I was thinking more along the lines of passing TMPDIR=/my/new/tmp/path > directly to sh in the runcmd call so it's only valid in the shell. Sounds reasonable. And there is no reason to use PATCHTOOL = "patch", I just think that it is a choice, and it should work, should it ? Well, if it's just a dead codepath that others no advantage over quilt, why should we even keep it? (In reply to comment #5) > Well, if it's just a dead codepath that others no advantage over quilt, why > should we even keep it? Agreed, let's try to drop a patch to oe-core mailing list. There is at least one still useful case where PATCHTOOL is set: commit a29c71024c304263bfb0732c870f20c1a6af7f64 Author: Martin Jansa <martin.jansa@gmail.com> Date: Wed Dec 4 18:33:18 2013 +0100 icecc-create-env-native: Drop FILESPATH, document PATCHTOOL * moving icecc-create-env to BPN allows to drop FILESPATH * document PATCHTOOL, because it's not easy to guess why it's needed With old archiver bbclasses I was using it too for most recipes: +# quilt leaves patches directories with symlinks to .patch files, revealing build paths in source archives +PATCHTOOL = "patch" +# fail to patch with "patch": +PATCHTOOL_pn-busybox = "quilt" +PATCHTOOL_pn-ruby = "quilt" +PATCHTOOL_pn-ruby-native = "quilt" Yes. There is a reason to use something else instead of quilt. Quilt does not support binary diffs. Git does, so I would need git to be used as patches tool. We're also using PATCHTOOL = "patch" in builds with archiver.bbclass, because quilt leaves patches directory which reveals build paths (I know we should rather extend archiver.bbclass to exclude them, but switching PATCHTOOL was simplest work around at that time). Pretty sure this was fixed with oe-core 51ea4378864f1468df2ca282a84f78a17d6861aa. Works on fido. Thanks. |