techtonik / python-patch

Library to parse and apply unified diffs
https://pypi.python.org/pypi/patch
112 stars 64 forks source link

Patch to enhance functionality #14

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
I'm using python-patch in one of my projects, so to return the favor here are 
my changes:

1. Support -p<num> like GNU patch, to strip path components

2. Set exit code to 0 or 1 depending on errors occured (like GNU patch)

3. Support creating new files (source=/dev/null), like GNU patch

4. Don't touch global logging configuration on module load, unless used as 
__main__  (useful when embedding in a larger project)

A patch is attached (applies cleanly to SVN r111)

Original issue reported on code.google.com by laa...@gmail.com on 27 Nov 2010 at 6:05

Attachments:

GoogleCodeExporter commented 9 years ago
New version of the patch, I had left a print debug statement in.

Original comment by laa...@gmail.com on 27 Nov 2010 at 6:44

Attachments:

GoogleCodeExporter commented 9 years ago
Thanks for the patch! However, it may worth to split it into several issues.

2. Return codes are on the way in TODO list, but with your request let's hope 
it will be committed faster. =) I'd like to find some time to cover apply() 
return values with tests.

1. -p<num> will be in place after Hg/Git patch format is added. The next thing 
after return codes. The Patch class is likely to be renamed to PatchSet before 
that.

2. New files support needs to well thought first, because if python-patch is 
used as a library, you probably don't want it to create files on your disk 
unless explicitly requested. issue 3 contains original request and issue 13 is 
how I am going to implement it if nothing else comes in mind.

4. The configuration should be local for "python_patch" module. Do you have a 
testcase that proves the global configuration is affected?

If you want to get more detailed feedback line-by-line regarding your patch 
code - upload it to http://codereview.appspot.com (you may also want to see 
http://pypi.python.org/pypi/review/ script).

Original comment by techtonik@gmail.com on 30 Nov 2010 at 5:48

GoogleCodeExporter commented 9 years ago
1. Yes, the return value scheme that I thought up is very simple; in my case I 
simply needed to know whether any of the patches had failed, in which case the 
installation process stops. For reporting it's probably better to also have a 
way to discover which patches failed.

2. I've indeed added -p<num> to cope with Git-generated patches.

3. Yes -- I've only added an option to overwrite or never overwrite files, but 
there also needs to be one to stop creating new files at all.
Same for removed files. apply() needs a lot more configuration.

4. Well there were two things I was doubtful about:

A:
loghandler = logging.StreamHandler()
logger.addHandler(loghandler)

B:
logger.setLevel(logging.CRITICAL)

A) Creates a handler, and attaches it to the handler chain for the logger. 
Doesn't strictly affect the global configuration, but why would a producer of 
log messages have to do this?

B) Changes the logging level for messages logged by patch. What if a program 
looks like this

def do_patch(...):
    import patch # sets logging level to CRITICAL, so that nothing gets logged
    p = patch.Patch(...)
    p.apply()

if __name__ == "__main__":
   # Set logging configuration from file
   logging.config.fileConfig("logging.conf")

   do_patch()

This application would first set up logging globally. This configuration file 
might contain directives to make 'patch.*' log all messages from DEBUG and up.

Then, a submodule of the projects uses 'patch' and actually imports it, which 
resets the logging level to CRITICAL.

I agree this is a difficult issue, because you want to provide a sane default, 
but the whole idea of introducing a global logging configuration is that there 
is no need to bother anymore with module-specific debug flags. Logging could be 
configurable even before importing any modules.

Original comment by laa...@gmail.com on 30 Nov 2010 at 9:43

GoogleCodeExporter commented 9 years ago
This issue was updated by revision r112.

Now it is possible to detect if patching of any file is failed.

Original comment by techtonik@gmail.com on 13 Dec 2010 at 11:51

GoogleCodeExporter commented 9 years ago
Because some points of this issue are partially resolved, it will be easier to 
track remaining questions in separate issues. If can create them then I'll 
close this one.

Original comment by techtonik@gmail.com on 9 Jan 2011 at 4:31

GoogleCodeExporter commented 9 years ago
This issue was updated by revision r150.

Solved questions around patch library logging when included in
larger projects. Many thanks! I've added a section to README
with explanations how to set defaults from an application.
http://code.google.com/p/python-patch/wiki/README#Library_usage

Original comment by techtonik@gmail.com on 7 Oct 2011 at 9:31

GoogleCodeExporter commented 9 years ago
This issue was updated by revision r155.

Added proper patch with simple test for stripping path components.

Original comment by techtonik@gmail.com on 29 Nov 2011 at 4:36

GoogleCodeExporter commented 9 years ago

Original comment by techtonik@gmail.com on 29 Nov 2011 at 4:37

techtonik commented 9 years ago

This issue needs more clarity.