Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions docs/markdown/Wrap-dependency-system-manual.md
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,34 @@ With such wrap file, `find_program('myprog')` will automatically
fallback to use the subproject, assuming it uses
`meson.override_find_program('myprog')`.

## Patch files

*Since: 0.59.0*

A list of local patch files can be provided in the `[patch-files]` section. They
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"patch-files" sounds extremely similar to patch_filename, which was, unwisely, not named "overlay_archive_filename". A while ago I lessened the confusion in the docs e.g. https://mesonbuild.com/Wrap-dependency-system-manual.html#wrapfile-with-meson-build-patch

For historic reasons this is called a "patch", however, it serves as an overlay to add or replace files rather than modifying them. The file must be an archive;

But we didn't/couldn't change the wrap file format's key name.

In order to prevent even more confusion, we cannot use "patch-file". What about "diff-file"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC (it was a long time ago) I opted for a new section specifically for avoiding confusion with patch_filename. Personally I still find it cleaner in a separate section.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

People will still just ask "do I add foo.patch to the section for patch files, or to the key for patch files?"

will be applied after the project has been extracted or cloned, and after the
`patch_filename` has been applied. `git` or `patch` command-line tool must be
available.

The `[patch-files]` section is optional and might contain the following keys:
- `strip`: Number of leading components from file names to strip. Defaults to 1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the strip level be able to be specified per patch?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, you're right and it's annoying because it makes the syntax more complicated :( Not sure it's worth the trouble, cerbero (from gst project) does not seems to be able to set strip for each patch, is it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any suggestion on the format we should use in the wrap file to allow per patch strip value?

One section per patch seems a bit cumbersome but the most future proof?

[patch 0001-foo.patch]

[patch 0002-bar.patch]
strip = 1

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or

[patch]
filename = 0001-foo.patch
[patch]
filename = 0002-bar.patch
strip = 1

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to standardize on -p1 as git format-patch / git diff create by default, and forbid specifying this manually. What is the use case for allowing it to be configurable?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, if we don't support configurable strip settings we don't need an entire section for this, we could just do:

[wrap-file]
diff_filename = ['0001-Change-return-value-to-43.patch',
                 '0002-Change-return-value-to-44.patch',
                 '0001-Change-foo-to-executable.patch',
                ]

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eli-schwartz strip should definitely defaults to 1. The use-case for having other value is for projects from another century that are not in git in the first place, like svn or pure tarball, in which case you could have patches with strip=0. But I think it actually makes no sense to mix strip levels, my original PR is actually fine I think.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would you have patches with strip=0? Just create them via diff -ur old/ new/ :p purely tarball based patching is a complete nightmare otherwise.

Or svn diff --git which adds a/ and b/ at the start.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a separate setting for strip is not necessary; as @eli-schwartz says, the user can convert a patch file, or regenerate it, if it's a non-standard legacy one.

diff_filename sounds like it mirrors patch_filename and source_filename. However, it accepts a list instead of a single filename. How about diff_filename_list? Or it could also accept an unquoted comma-separated list, which would make it compatible with patch_filename and source_filename (when providing a single element).

if omitted.
- `patches`: A list of patch filenames relative to `subprojects/packagefiles`
directory. It must be in the python string list format (e.g. `['foo', 'bar']`).

```ini
[wrap-file]
directory = libfoobar-1.0

source_url = https://example.com/foobar-1.0.tar.gz
source_filename = foobar-1.0.tar.gz
source_hash = 5ebeea0dfb75d090ea0e7ff84799b2a7a1550db3fe61eb5f6f61c2e971e57663

[patch-files]
strip = 1
patches = ['0001.patch', '0002.patch']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These patch files need to have checksums as well.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it's only local files, not something we download.

```

## Using wrapped projects

Wraps provide a convenient way of obtaining a project into your
Expand Down
18 changes: 18 additions & 0 deletions docs/markdown/snippets/patch_files.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
## Wrap files can contain a list of patch files to apply

A list of local patch files can be provided by the `.wrap` file and they will be
applied after the subproject has been extracted or cloned from git. This requires
the `patch` or `git` command-line tool.

```ini
[wrap-file]
directory = libfoobar-1.0

source_url = https://example.com/foobar-1.0.tar.gz
source_filename = foobar-1.0.tar.gz
source_hash = 5ebeea0dfb75d090ea0e7ff84799b2a7a1550db3fe61eb5f6f61c2e971e57663

[patch-files]
strip = 1
patches = ['0001.patch', '0002.patch']
```
37 changes: 36 additions & 1 deletion mesonbuild/wrap/wrap.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,12 @@
import configparser
import typing as T
import textwrap
import ast

from pathlib import Path
from . import WrapMode
from .. import coredata
from ..mesonlib import quiet_git, GIT, ProgressBar, MesonException
from ..mesonlib import quiet_git, GIT, ProgressBar, MesonException, Popen_safe
from .. import mesonlib

if T.TYPE_CHECKING:
Expand All @@ -51,6 +52,8 @@

ALL_TYPES = ['file', 'git', 'hg', 'svn']

PATCH = shutil.which('patch')

def whitelist_wrapdb(urlstr: str) -> urllib.parse.ParseResult:
""" raises WrapException if not whitelisted subdomain """
url = urllib.parse.urlparse(urlstr)
Expand Down Expand Up @@ -96,6 +99,7 @@ def __init__(self, fname: str):
self.values = {} # type: T.Dict[str, str]
self.provided_deps = {} # type: T.Dict[str, T.Optional[str]]
self.provided_programs = [] # type: T.List[str]
self.patches: T.List[str] = []
self.basename = os.path.basename(fname)
self.has_wrap = self.basename.endswith('.wrap')
self.name = self.basename[:-5] if self.has_wrap else self.basename
Expand Down Expand Up @@ -139,6 +143,7 @@ def parse_wrap(self) -> None:
self.parse_wrap()
return
self.parse_provide_section(config)
self.parse_patch_files_section(config)

def parse_wrap_section(self, config: configparser.ConfigParser) -> None:
if len(config.sections()) < 1:
Expand Down Expand Up @@ -171,6 +176,12 @@ def parse_provide_section(self, config: configparser.ConfigParser) -> None:
raise WrapException(m.format(k, self.basename))
self.provided_deps[k] = v

def parse_patch_files_section(self, config: configparser.ConfigParser) -> None:
if config.has_section('patch-files'):
values = config['patch-files']
self.patches = ast.literal_eval(values.get('patches', '[]'))
self.strip = int(values.get('strip', '1'))

def get(self, key: str) -> str:
try:
return self.values[key]
Expand Down Expand Up @@ -582,6 +593,30 @@ def apply_patch(self) -> None:
if not os.path.isdir(src_dir):
raise WrapException(f'patch directory does not exists: {patch_dir}')
self.copy_tree(src_dir, self.dirname)
for i in self.wrap.patches:
from ..interpreterbase import FeatureNew
FeatureNew('patch-files section', '0.59.0').use(self.current_subproject)
mlog.log(f'Applying patch {i}')
fname = os.path.join(self.wrap.filesdir, i)
if GIT:
if self.wrap.type == 'git':
# Assume patches are made with `git format-patch`
cmd = [GIT, 'am', fname]
else:
# `git apply` can be used outside of a git repository, but
# when used inside a git repository paths are assumed to be
# relative to root git dir instead of current working dir.
# Override that behaviour by using --work-tree.
cmd = [GIT, '--work-tree', self.dirname, 'apply', fname]
elif PATCH:
cmd = [PATCH, '-f', '-i', fname]
else:
raise WrapException('Missing "git" or "patch" commands to apply patch files')
cmd += ['-p' + str(self.wrap.strip), '--ignore-whitespace']
p, o, e = Popen_safe(cmd, cwd=self.dirname, stderr=subprocess.STDOUT)
if p.returncode != 0:
mlog.log(o.strip())
raise WrapException('Failed to apply patch')

def copy_tree(self, root_src_dir: str, root_dst_dir: str) -> None:
"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,8 @@ executable('grabprog2', files('src/subprojects/foo/prog2.c'))
subdir('src')

subproject('patchdir')

if find_program('patch', required : false).found() or find_program('git', required : false).found()
exe = subproject('patchfile').get_variable('foo_exe')
test('test_foo', exe)
endif
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
From b79f6cc4a096f6c2888f73b947b652491885896a Mon Sep 17 00:00:00 2001
From: Xavier Claessens <[email protected]>
Date: Fri, 30 Nov 2018 14:13:47 -0500
Subject: [PATCH] Change foo to executable

---
foo.c | 4 ++++
meson.build | 2 +-
2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/foo.c b/foo.c
index 54f9119..468f033 100644
--- a/foo.c
+++ b/foo.c
@@ -1,3 +1,7 @@
int dummy_func(void) {
return 44;
}
+
+int main(void) {
+ return dummy_func() == 44 ? 0 : 1;
+}
diff --git a/meson.build b/meson.build
index 318e81d..4a281d9 100644
--- a/meson.build
+++ b/meson.build
@@ -1,2 +1,2 @@
project('static lib patchdir', 'c')
-libfoo = static_library('foo', 'foo.c')
+foo_exe = executable('foo', 'foo.c')
--
2.17.1

Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
From 7001dcc738e5ae7dfa8af20ed582b9a985804f72 Mon Sep 17 00:00:00 2001
From: Xavier Claessens <[email protected]>
Date: Fri, 30 Nov 2018 10:15:33 -0500
Subject: [PATCH 1/2] Change return value to 43

---
foo.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/foo.c b/foo.c
index 019f2ba..e4577b8 100644
--- a/foo.c
+++ b/foo.c
@@ -1,3 +1,3 @@
int dummy_func(void) {
- return 42;
+ return 43;
}
--
2.17.1

Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
From c2da2e490b09f2e251c7f4ef7c1240acee215fec Mon Sep 17 00:00:00 2001
From: Xavier Claessens <[email protected]>
Date: Fri, 30 Nov 2018 10:15:47 -0500
Subject: [PATCH 2/2] Change return value to 44

---
foo.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/foo.c b/foo.c
index e4577b8..54f9119 100644
--- a/foo.c
+++ b/foo.c
@@ -1,3 +1,3 @@
int dummy_func(void) {
- return 43;
+ return 44;
}
--
2.17.1

Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
[wrap-file]
directory = foo-1.0-patchfile

source_url = http://something.invalid
source_filename = foo-1.0.tar.xz
source_hash = 9ed8f67d75e43d3be161efb6eddf30dd01995a958ca83951ea64234bac8908c1
lead_directory_missing = true

patch_directory = foo-1.0

[patch-files]
patches = ['patchfile/0001-Change-return-value-to-43.patch',
'patchfile/0002-Change-return-value-to-44.patch',
'patchfile/0001-Change-foo-to-executable.patch',
]