libfossil

FIXMEs
Login

FIXMEs

An informal list of FIXMEs which don't warrant full tickets...

This is not intended to be a list of long-lived fixme's, but more of a scratchpad for things which come up spontaneously and shouldn't be forgotten.

In no particular order...

Diff Engine v2

Related to the "v2" diff engine (2021 edition)...

Diff formats with chunk headers sometimes split unnecessarily

Ticket [746ebbe86c20b5c0f96cdadd19abd8284770de16].

When outputing to a format which uses "chunk headers" (like unified) can sometimes split two chunks which are directly adjacent:

$ ./f-vdiff 072d63965188 a725befe5863 -l '*vdiff*' | head -30
Index: f-apps/f-vdiff.c
==================================================================
--- f-apps/f-vdiff.c
+++ f-apps/f-vdiff.c
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    36     36    fsl_buffer fname;
    37     37    fsl_buffer fcontent1;
    38     38    fsl_buffer fcontent2;
    39     39    fsl_buffer fhash;
    40     40    fsl_list globs;
           41 +  fsl_diff_opt diffOpt;
           42 +  fsl_diff_builder * diffBuilder;
    41     43  } VDiffApp = {
    42     44  NULL/*glob*/,
    43     45  5/*contextLines*/,
    44     46  0/*sbsWidth*/,
    45     47  0/*diffFlags*/,
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    46     48  0/*brief*/,
    47     49  fsl_buffer_empty_m/*fname*/,
    48     50  fsl_buffer_empty_m/*fcontent1*/,
    49     51  fsl_buffer_empty_m/*fcontent2*/,
    50     52  fsl_buffer_empty_m/*fhash*/,
    51        -fsl_list_empty_m/*globs*/
           53 +fsl_list_empty_m/*globs*/,
           54 +fsl_diff_opt_empty_m/*diffOpt*/,
           55 +NULL/*diffBuilder*/
    52     56  };
    53     57  
    54     58  static int f_vdiff_hash(fsl_card_F const * const fc,

Note how line numbers above and below the ~~~~~ (diff chunk separator) are consecutive. fossil(1) doesn't do that because it handles the chunk separator at a higher level of the API, whereas libf moves that into the diff builder interface. That change requires that libf forego a step fossil performs which tries to merge chunks of a diff together if the distance between them is smaller than the number of context lines. That change in libf is apparently the root cause of this. It's purely cosmetic, but it's annoying. The core of the discrepancy comes from fossil mixing the output of of the diffs: most of it is done from the diff builders but some pieces, like Unified-format diff line number markers, are output from the core diff driver algorithm. That gives it more control over the output, but it also only supports a single output channel (a memory buffer), whereas libf needs to be able to support arbitrary output channels (some of which, like ncurses widgets, cannot simply be streamed-to).

TODO: figure out how to adapt libf to (A) not require that the higher-level API know such details about the diff builder but (B) still be able to apply that "cosmetic optimization."

Checkin 08fdbacefacd2b partially resolves this, but only for the case when we're using line numbers (as in the above example).

The Unusually and Excessively Large Diff

Update 2022-01-20: the cause of this has been narrowed down to a broken attempt in f-vdiff to report a rename, which cascades into all sorts of grief. Until this can be solved properly, f-vdiff will report a rename as an add/remove pair. Not ideal, but it's the quickest fix for the time being.

Here's a case where libf's diff (via f-vdiff) produces a multi-hundred-meg diff compared to half a meg for fossil...

To reproduce, from the main fossil tree:

$ fossil co c800d2ca827c1fe5f
$ fossil merge 9769c4f7563  # results are the same with f-merge
$ f-vdiff -f u > X          # (--from 9769c4f75639) gives similar results
$ fossil diff --unified > Y # (--from 9769c4f75639) gives much smaller results
$ ls -la X Y
-rw-rw-r-- 1 stephan stephan 282218662 Jan 15 10:34 X
-rw-rw-r-- 1 stephan stephan    584208 Jan 15 10:35 Y

!?!?!? Results are similar with other diff formats.

Backports from Fossil

This list is for potential backports from fossil which should be investigated more closely to determine whether they need to be adapted for use in this library:

f-apps-specific

f-vdiff

Recognize Renames

f-vdiff, when comparing to the local checkout, needs to be able to recognize renames. Case in point:

[stephan@nuc:~/fossil/fossil]$ f-vdiff . . 
f-vdiff: ERROR #107 (FSL_RC_NOT_FOUND): Cannot stat file; /home/stephan/fossil/fossil/src/codecheck1.c

That file, along with many others, was moved to ../tools in this checkout. We have the state necessary to do this check (namely, the vfile table), it just needs to get done).

Optionally show full content of added/removed files

For newly-added/removed files, f-vdiff politely only shows "added" or "removed," not the whole content. Thought it's ostensibly easy to add an option to do this, f-vdiff's internal structure didn't really foresee that possibility and retrofitting it now requires some reworking.