Edgewall Software
Modify

Opened 12 years ago

Last modified 3 years ago

#6682 new defect

"binary files ... differ" in diff/patch files

Reported by: r.bhatia@… Owned by:
Priority: normal Milestone: next-stable-1.2.x
Component: rendering Version: 0.10.3
Severity: minor Keywords: diff patch
Cc: Branch:
Release Notes:
API Changes:

Description

hi, please do not skip content of patchfiles when it states that two binary files differ:

e.g.

--- a/main.inc.php	2007-11-29 09:45:26.065072250 +0100
+++ b/main.inc.php	2008-01-14 22:48:30.340162500 +0100
@@ -85,7 +85,7 @@ $rcmail_config['smtp_log'] = TRUE;
 $rcmail_config['list_cols'] = array('subject', 'from', 'date', 'size');
 
 // relative path to the skin folder
-$rcmail_config['skin_path'] = 'skins/default/';
+$rcmail_config['skin_path'] = 'skins/ipax/';
 
 // use this folder to store temp files (must be writebale for apache user)
 $rcmail_config['temp_dir'] = 'temp/';
Binary files a/blank.gif and b/blank.gif differ

is it possible to render such information at the top, when displaying the content of patch files?

cheers,

Attachments (0)

Change History (12)

comment:1 by r.bhatia@…, 12 years ago

ps. perhaps this is no proper diff file - but to my knowledge it applies well :)

in reply to:  description comment:2 by Emmanuel Blot, 12 years ago

Component: generalchangeset view
Keywords: needinfo added
Owner: changed from Jonas Borgström to Christian Boos

Replying to r.bhatia@ipax.at:

hi, please do not skip content of patchfiles when it states that two binary files differ:

You cannot "diff" binary files: diff/patch tool suite cannot work binary files (diff would only be able to report that files are different). Several tools exist to create delta fro binary files, but this is OOT.

The example you provided shows PHP files, i.e. non-binary files. Why do you report them as binary? It is a configuration issue within your Subversion repository?

comment:3 by Christian Boos, 12 years ago

Keywords: needinfo removed
Milestone: 0.11.1
Severity: normalminor

I think he was talking about that line:

Binary files a/blank.gif and b/blank.gif differ

which is probably skipped in 0.10.x:

  • main.inc.php

    a b $rcmail_config['smtp_log'] = TRUE;  
    8585$rcmail_config['list_cols'] = array('subject', 'from', 'date', 'size');
    8686
    8787// relative path to the skin folder
    88 $rcmail_config['skin_path'] = 'skins/default/';
     88$rcmail_config['skin_path'] = 'skins/ipax/';
    8989
    9090// use this folder to store temp files (must be writebale for apache user)
    9191$rcmail_config['temp_dir'] = 'temp/';

Right.

I would have expected this to work in 0.11, but no. And btw, the diffs inlined in Wiki pages seem to be broken atm. (404 for diff.css?)

in reply to:  3 comment:4 by Christian Boos, 12 years ago

Replying to cboos:

And btw, the diffs inlined in Wiki pages seem to be broken atm. (404 for diff.css?)

For some reason, the add_stylesheet(req, 'common/css/diff.css') in the patch.py code translates to $.loadStyleSheet("/devel/chrome/site/css/diff.css", ""); in the generated HTML…

in reply to:  3 comment:5 by r.bhatia@…, 12 years ago

Replying to cboos:

I think he was talking about that line:

Binary files a/blank.gif and b/blank.gif differ

I would have expected this to work in 0.11, but no.

right, that is what i'm talking about. i know that binary files cannot be "diff"ed but when someone checks in a file with this content, trac will never show those lines.

maybe the correct solution is to display some kind of an error-log for all lines which are skipped by the PatchRenderer class.

thanks for replying so fast!

cheers, raoul

comment:6 by Christian Boos, 11 years ago

Component: version control/changeset viewrendering
Keywords: diff added

comment:7 by Christian Boos, 11 years ago

Milestone: 0.11.20.12

I've got a few pending enhancements to the patch renderer, including minimal support for Git style binary patches. I should add support for the "Binary files a/blank.gif and b/blank.gif differ" case as well.

comment:8 by Remy Blank, 10 years ago

Milestone: 0.12next-minor-0.12.x

Not a blocker for 0.12, but feel free to move back to 0.12 if you have time to refresh this.

comment:9 by Ryan J Ollos, 5 years ago

Milestone: next-minor-0.12.xnext-stable-1.0.x

comment:10 by Ryan J Ollos, 4 years ago

Owner: Christian Boos removed

comment:11 by figaro, 4 years ago

Keywords: patch added

comment:12 by Ryan J Ollos, 3 years ago

Milestone: next-stable-1.0.xnext-stable-1.2.x

Moved ticket assigned to next-stable-1.0.x since maintenance of 1.0.x is coming to a close. Please move the ticket back if it's critical to fix on 1.0.x.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as new The ticket will remain with no owner.
The ticket will be disowned. Next status will be 'new'.
as The resolution will be set. Next status will be 'closed'.
The owner will be changed from (none) to anonymous. Next status will be 'assigned'.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.