Skip to content

tools.patch fails with new/deleted files #1807

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

Closed
theodelrieu opened this issue Sep 27, 2017 · 12 comments
Closed

tools.patch fails with new/deleted files #1807

theodelrieu opened this issue Sep 27, 2017 · 12 comments
Assignees
Milestone

Comments

@theodelrieu
Copy link
Contributor

Hello,

I tried to apply a .patch using tools.patch, however the dependency conan is using does not handle new/deleted files. (see this line in particular).

Furthermore, this library has not been updated in a year. This one looks maintained, I haven't tested it though.

To workaround that, I used self.run to launch git apply.

@memsharded
Copy link
Member

Thanks for having a look at this, and the suggestion. I think we could try to use the proposed, maintained one, lets have a look.

@memsharded
Copy link
Member

Could you please give a failing patch to apply? I guess it will be the typical ---- null, ++++new_file, right? We should add some test.

@theodelrieu
Copy link
Contributor Author

Indeed, here is a minimum patch file that reproduces the error:

diff --git a/newfile b/newfile
new file mode 100644
index 0000000..ea450f9
--- /dev/null
+++ b/newfile
@@ -0,0 +1 @@
+New file

@memsharded memsharded self-assigned this Oct 10, 2017
@memsharded
Copy link
Member

The https://github.com/vrajat/git-patch repo is just a wrapper over git, I think it is better to use a python solution, for those not using git.

I have tried to contribute upstream: techtonik/python-patch#52

@theodelrieu
Copy link
Contributor Author

I agree. Not sure the repo is still maintained though, looking at the last closed issue which was in September 2016.

Also I remember the patch command failed at the first warning, that might be why your PR doesn't pass.

@memsharded
Copy link
Member

I have proposed a local fix in: #1898

The upstream is abandoned, no response from PR. Also there is already a PR for that issue long time ago, without response

Cannot find viable alternatives. Tried google patch-diff lib, but only from text-text no real unidiff with new and deletes.

@theodelrieu
Copy link
Contributor Author

That's unfortunate... What about forking the repo? I don't see many options anyway.

@memsharded
Copy link
Member

Released in 0.28

@theodelrieu
Copy link
Contributor Author

Hi @memsharded, I've tried to use tools.patch again, and it still fails with the following message:

WARN: sioclient.patch: error: absolute paths are not allowed - file no.3
WARN: sioclient.patch: stripping absolute path from source name 'b'/dev/null''

Was the forked patch dependency published to pip?

@memsharded
Copy link
Member

No, finally conan implemented its own management of creation/deletion of files. Check it here. https://github.com/conan-io/conan/blob/develop/conans/client/tools/files.py#L174

There are conan tests covering this functionality: https://github.com/conan-io/conan/blob/develop/conans/test/util/conanfile_tools_test.py#L95

Maybe you could try to provide a failing test to replicate this issue? What is exactly your patch? Thanks for the feedback

@theodelrieu
Copy link
Contributor Author

theodelrieu commented Nov 11, 2017

Sure, here is a recipe that should reproduce the bug.

new_file_bug.zip

Sorry about that, it seems the warnings are not related to the failure I had earlier, the build passes with the warnings.

I thought I saw in the patch.py code that any warning prevented the patch to be applied.

Anyways it works :)

@memsharded
Copy link
Member

Ok, great, glad that it is working, thanks for telling :)

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

No branches or pull requests

3 participants