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

[feature] Conan 1.40 (PR-9361) breaking changes #9587

Closed
sboehmann opened this issue Sep 11, 2021 · 5 comments
Closed

[feature] Conan 1.40 (PR-9361) breaking changes #9587

sboehmann opened this issue Sep 11, 2021 · 5 comments
Assignees
Milestone

Comments

@sboehmann
Copy link

Sorry about the sloppy title. ;)

We used to have our own method, but then ported to apply_conandata_patches(). This worked fine, but with conan 1.40 (due #9361) all our recipes are broken.

Sure, - this feature is still experimental, so I had to expect something to break during the update. But this PR also removes the possibility to specify a base_path. We use this option intensively and it is very useful. We try to take unmodified patches from upstream. But to apply them you often need to set base_path (and/or strip).

@memsharded
Copy link
Member

Hi @sboehmann

The possibility is encoded now in the source folder of the new layout. This change had to be done because the feature was broken when using the layouts, due to the duplicated and often incompatible definition.

You should be able to define and make it work with something like:

def layout(self):
        self.folders.source = "mysrc_path"

Could you please have a look and see if that works for you?

From what I see in the diff to 1.39, the strip argument hasn't changed, it is still available in the patch() function, and it was not available in direct arg in apply_conandata_patches() in 1.39, the signature of the method hasn't changed here, so the strip should keep working as it was. If not, please clarify, we could do a patch for next 1.40.1 patch release. Thanks!

@memsharded memsharded added this to the 1.40.1 milestone Sep 12, 2021
@sboehmann
Copy link
Author

sboehmann commented Sep 12, 2021

Thank you for your quick reply @memsharded.

Regarding patch():

The pull request changes two things.

One is the base directory (the root argument for patchset.apply). I assume this will work again for me if I implement layout() according to your suggestion. This change makes sense and I am perfectly fine with it.

The second change is that the base_path argument has been removed. Only this part is a problem for me. I would have to modify a lot of patches now because I can't specify different base_path anymore. But I try to use unmodified patches provided by upstream. For these patches I often have to use a different base_path. And if I have multiple patches for the same recipe, it may even differ per patch.

My feature request is therefore to add base_path back to the patch method (and if set, join conanfile.source_folder and base_path).

Regarding apply_conandata_patches():

Sorry, I misremembered. Contrary to what I wrote above, I don't use this method. I wanted to use it, but had to implement it myself to be able to specify the strip argument.

So my second feature request, although not quite as important, would be to restore base_path here as well, but also to be able to specify the strip options in conandata.yml.

@memsharded
Copy link
Member

Ok, yes, I was a bit confused about the method, if you were talking about the patch() one, that could make more sense.

Lets ask @lasote for this, he implemented this change, maybe there is another approach, or maybe the base_path makes sense.

@lasote
Copy link
Contributor

lasote commented Sep 13, 2021

Hi, I understand. I will open a PR to use again the base_path as relative to the conanfile.source_folder. If a layout is specified, it will use it, otherwise, it will use the base source folder as previously.

About the strip argument, I would say if specified in the conandata.yml, it will pass it to strip automatically like any other argument.

@memsharded
Copy link
Member

Feature was added in #9593, will be in next 1.40.1 soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants