Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

specify patches in conandata.yaml #90

Closed
SSE4 opened this issue Sep 25, 2019 · 13 comments · Fixed by conan-io/hooks#109
Closed

specify patches in conandata.yaml #90

SSE4 opened this issue Sep 25, 2019 · 13 comments · Fixed by conan-io/hooks#109
Labels
enhancement New feature or request

Comments

@SSE4
Copy link
Contributor

SSE4 commented Sep 25, 2019

it's often needed to specify patches to apply, and list of patches may vary from version to version.
we may use something like:

patches:
    1.70.0:
        - "001-xxx"
        - "002-xxxx"

it also would be nice to ensure every recipe uses the same format. consider use some sort of schema to validate conandata.yml has only allowed fields.

@lasote lasote added the enhancement New feature or request label Sep 25, 2019
@lasote
Copy link
Contributor

lasote commented Sep 25, 2019

Could you provide how the patch looks like in the recipe using the conandata, might we consider some improvement?

@SSE4
Copy link
Contributor Author

SSE4 commented Sep 25, 2019

current code in boost:

    def source(self):
        patches = self.conan_data["patches"][self.version]["patches"]
        patches = patches.split(",") if patches else []
        tools.get(**self.conan_data["sources"][self.version])
        for patch in patches:
            tools.patch(patch_file=os.path.join("patches", patch),
                        base_path=os.path.join(self.source_folder, self._folder_name))

with YML:

patches:
  1.70.0:
    patches: "0001-beast-fix-moved-from-executor.patch,bcp_namespace_issues.patch,boost_build_qcc_fix_debug_build_parameter.patch,boost_core_qnx_cxx_provide___cxa_get_globals.patch,python_base_prefix.patch"
  1.71.0:
    patches: "bcp_namespace_issues.patch,boost_build_qcc_fix_debug_build_parameter.patch,boost_core_qnx_cxx_provide___cxa_get_globals.patch,python_base_prefix.patch"

@lasote
Copy link
Contributor

lasote commented Sep 25, 2019

@conan-io/barbarians Let's decide something here about this.
Any objection about specifying patches in the conandata.yml?
Agreed with the format?

patches:
    1.70.0:
        - "001-xxx"
        - "002-xxxx"

@memsharded about our conversation about "peeling" the conandata being exported to avoid creating new recipe revisions when only new version keys are added we would be ok, but only if we agree on the "standard" key and introduce checks of known keys in the CI.

@jgsogo
Copy link
Contributor

jgsogo commented Sep 25, 2019

Each item in the list of patches: is just a filename? is there a directory for patches? relative path from conanfile.py to the patch?

@SSE4
Copy link
Contributor Author

SSE4 commented Sep 25, 2019

another option - consider adopting patch series file approach (used by debian) https://linux.die.net/man/1/quilt

@jgsogo
Copy link
Contributor

jgsogo commented Sep 25, 2019

another option - consider adopting patch series file approach (used by debian) https://linux.die.net/man/1/quilt

I think it is overkill, don't you think so?

@uilianries
Copy link
Member

@conan-io/barbarians Let's decide something here about this.
Any objection about specifying patches in the conandata.yml?
Agreed with the format?

patches:
    1.70.0:
        - "001-xxx"
        - "002-xxxx"

I agree, since we don't use some naming restriction. e.g. We only accept patch files using the follow pattern: "\d\d\d-.*.patch"

@lasote lasote assigned lasote and unassigned memsharded Sep 27, 2019
@SSE4
Copy link
Contributor Author

SSE4 commented Oct 1, 2019

let's use the proposed format then (KISS) - looks simple and neat
in future maybe adding some checks to our conan center hook that patches are in right place and have right names (so they are always applied in right order)
P.S. .diif is also very common in addition to the .patch

@danimtb
Copy link
Member

danimtb commented Oct 1, 2019

I like the approach and this also includes a "standard" patch folder to store the patches with the names indicated. That way this could evolve in the future into a built-in Conan tool

@lasote
Copy link
Contributor

lasote commented Oct 2, 2019

We propose this in case the patch is remote or local. The function to process this should be documented as a snippet, we could think later about creating a specific tool.

patches:
    1.70.0:
        -  "patch_file": "001-xxx"
           "base_path": "source_subfolder" 
        -  "url": XXX
           "checksum": XXX
           "base_path": "source_subfolder"

@uilianries
Copy link
Member

When a package requires only one patch, you could optimize:

# conandata.yml
patches:
  "8.0.17":
    base_path: "source_subfolder"
    patch_file: "patches/0001-find-cmake.patch"
# conanfile.py
def build(self):
    tools.patch(**self.conan_data["patches"][self.version])

@Croydon
Copy link
Contributor

Croydon commented Oct 3, 2019

Maybe we should keep it uniform no matter if it is a single patch or several.

By that way we would always get a list back or could maybe improve tools.patch() to support this as well. This would allow to manage patches truly in conandata.yml without code changes (adding or removing a loop like for patch in patches:)

@lasote
Copy link
Contributor

lasote commented Oct 7, 2019

As I said: we could think later about creating a specific tool. Let's start with everything manual.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants