#107 A potential bug in align_by_pairs() ?

[tksh]

Hi alnd,

Sorry if I'm mistaken or it's a FAQ,
but I believe your align_by_pairs() implementation in cloc-1.60.pl is strange
and has a big potential bug !

Consider the basic use case of 'cloc --diff',
i.e. 'diff between two directories org/ and mod/',
then suppose the following file-tree:

   /root/
     |-- org/
     |   |- src/
     |   |   |- main.c
     |   |   |- subc.c
     |
     |-- mod/
     |   |- common.h    # <= new file here
     |   |- src/
     |   |   |- main.c  # no change
     |   |   |- subc.c  # no change

Then execute 'cloc --diff':

    cd root
    cloc --diff  org/  mod/

Obviously the result should be
'same files=2, modified=0, added=1, deleted=0',
but the real result will be
'same files=0, modified=0, added=3, deleted=2'.

It's because remove_leading_dir() removes 'org/src/' from files under org/,
but removes only 'mod/' from files under mod/.

The above might be a rare case, but would sometimes occur,
and the result would be catastrophic !

Why don't you compare file paths between two file-trees,
simply using 'relative path from each specified top dir'
(org/ and mod/ in this case) ?

Thank you in advance,
tksh

[Al Danial]

A new diff alignment algorithm appears with svn commit 390. It fixes this problem but I'll need to do more testing with different setups.

[tksh]

Thank you for trying fix,
but r390 seems to produce a wrong result for '--diff org/ added-dir/' below:

org/
 |-- dir/
     |-- 01.c
     |-- 02.c

added-dir/
 |-- dir/
     |-- 01.c
     |-- 02.c
     |-- newdir/
         |-- new.c

The result of 1.60 is '2 same, 1 added files' (OK),
but r390 outputs '3 added, 2 removed filed' (NG).

In addition, r390 will go into an infinate loop when executed with '--diff org/ org/'.

[Al Danial]

Thanks for the update. I'll work on a better algorithm.

[tksh]

My adhoc patch is attached for reference.
Specify --diffdir instead of --diff as follows, to enable the fix.

perl cloc-1.60-fix107.2.p --skip-uniqueness --diffdir <dir1> <dir2></dir2></dir1>

[Al Danial]

I look forward to looking at your new code and seeing how well it does the alignment. Currently I'm traveling so it will be a few days before I get to it. This bug (and related https://sourceforge.net/p/cloc/bugs/98/) are the main hold-ups to the next release so I'm eager to have it fixed.

[Al Danial]

Your patched code works fine when the inputs are two directories, but cloc also needs to work with pairs of archive files, eg org.tar and added_dir.tar, or an archive and a directory. In these cases identifying the correct $TopDirL and $TopDirR is a bit more involved. Still, you provided a valuable insight in that the burden should be on the user to supply compatible inputs rather than making cloc figure this out. I'll work your idea into cloc.

[Al Danial]

The tar v. directory case proved too messy to handle with the logic I had in mind when I wrote the previous post so I reworked another method to determine $TopDirL and $TopDirR. svn commit 391,
https://sourceforge.net/p/cloc/code/HEAD/tree/trunk/cloc, looks like it handles your cases well. Please try it if you get a chance.

[tksh]

Sorry for late response.

Your fix may work well in most cases, but same as before, it could fail in some case.
It's theoretically impossible to figure out leading dirs with 100% certainty
only based on file-paths themselves, because it's a matter of user's choice.

When the inputs are archive files, you can handle them same as directories:

• Add an option for an archive file, which designates the internal dir path to be regarded as the leading directory. For example, org.tar:module/src

• Suppose org.tar is expanded under /tmp/tempdir, then you can identify
  the leading dir of org.tar:module/src with /tmp/tempdir/module/src

[Al Danial]

cloc is overdue for a new release so I won't make more changes until 1.62 is out the door.

If you create a case where the current solution fails, please post it as a new bug report and I'll sharpen the logic.

[Al Danial]

v1.62 released