Fossil Forum

Bug seemingly introduced in 'diff-color-enhancements' branch
Login

Bug seemingly introduced in 'diff-color-enhancements' branch

Bug seemingly introduced in 'diff-color-enhancements' branch

(1) By jshoyer on 2022-02-25 22:27:27 [link] [source]

The following is giving me a puzzling result with fossil 2.18 but not 2.17:

fossil new reprex-diff-bug.fossil
fossil open reprex-diff-bug.fossil --workdir working_directory
cd working_directory
echo "Document an accepted paper" > file.txt
fossil add file.txt
fossil ci -m 'Add one-line file'
echo "Document UCBSV multispectral imaging paper" > file.txt
fossil diff -by

For reasons that I have not yet figured out, the right-hand side of the resulting diff shows several extra lines, each progressively truncated by an additional character (and with odd highlighting):

Document UCBSV multispectral imaging paper
CBSV multispectral imaging paper
BSV multispectral imaging paper
SV multispectral imaging paper
V multispectral imaging paper
 multispectral imaging paper
multispectral imaging paper

I get the puzzling diff with fossil built from check-in 9e330740 (in which the 'diff-color-enhancements' branch was merged in to trunk) but not from trunk check-in c717d280 (the parent of that branch). https://fossil-scm.org/home/timeline?r=diff-color-enhancements

I'll try to bisect the issue further within the 'diff-color-enhancements' branch.

(2) By Larry Brasfield (larrybr) on 2022-02-25 23:03:21 in reply to 1 [link] [source]

I ran the OP's repro in /tmp, and got a normal looking, side-by-side diff display in the browser. However, I also got this at the console: $ fossil diff -by libva error: /usr/lib/x86_64-linux-gnu/dri/iHD_drv_video.so init failed [399006:399006:0100/000000.928864:ERROR:sandbox_linux.cc(377)] InitializeSandbox() called with multiple threads in process gpu-process. $ fossil version This is fossil version 2.18 [84f25d7eb1] 2022-02-23 13:22:20 UTC $ uname -a Linux Bit-Borker 5.13.0-28-generic #31-Ubuntu SMP Thu Jan 13 17:41:06 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux .
Maybe this yields a clue regarding the OP's result.

(3) By Stephan Beal (stephan) on 2022-02-25 23:05:50 in reply to 2 [link] [source]

Maybe this yields a clue regarding the OP's result.

That message is from the browser. Both Chromium and Firefox have their own set of adorable warnings which spew out when running fossil ui and the like.

(4.1) By jshoyer on 2022-02-26 17:37:02 edited from 4.0 in reply to 2 [link] [source]

Larry — thank you for trying to reproduce the issue.

I think that the bug is somehow dependent on the build environment. I get the error on an x64 Macbook running Monterey 12.1 but not an older Macbook running Sierra 10.12.6. I do however get the error on the older machine with the official 2.18 binary downloaded from the website.

I think the issue was introduced in check-in 4cd8a743cd0c1f96 — I do not get the same issue with its parent check-in, ce856a86 (which yields a slightly odd diff for me, but not the repetition issue).

(5) By jshoyer on 2022-03-03 23:36:49 in reply to 4.1 [link] [source]

I tested this on two more machines. Did not have issue on CentOS 7 (with v2.18 Linux x64 binary from the website).

A third mac, running Sierra 10.12.6, behaved like the other machine running that version of the OS:

  • had issue with 2.18 macOS x64 binary downloaded from the website
  • no issue with 2.18 built from source
  • no issue with 2.17 binary downloaded from the website

It seems possible that the bug is conditional on differences in old vs new Mac developer tools/libraries, but I do not know why that would be. I have not yet studied the changes from check-in 4cd8a743cd0c1f96 to guess what might be happening, but I will try to do that.

I have also noticed that I sometimes get more extreme diff errors (more lines duplicated) with `fossil diff -by` than with `fossil diff -tk`, but I have not come up with a minimal example for that yet.

(6.1) By Stephan Beal (stephan) on 2022-03-03 23:49:19 edited from 6.0 in reply to 5 [link] [source]

A third mac, running Sierra 10.12.6, behaved like the other machine running that version of the OS:

Just coincidentally... about 90 seconds before you posted, Martin G. posted what might be the same problem in the libfossil chat. To reproduce it, apply the patch below to the current fossil trunk, then run (fossil diff -by). On his Mac the output is utterly mysteriously broken in a way which reminds me of your problem. On my machines (Linux) it looks exactly as expected.

Edit: to apply the patch run this from the top of a fossil trunk checkout: patch -p0 < /the/patch/below

Index: src/file.c
==================================================================
--- src/file.c
+++ src/file.c
@@ -1298,55 +1298,13 @@
 ** a full pathname.  The result is obtained from fossil_malloc()
 ** and should be freed by the caller.
 */
 char *file_fullexename(const char *zCmd){
 #ifdef _WIN32
-  char *zPath;
-  char *z = 0;
-  const char *zExe = "";
-  if( sqlite3_strlike("%.exe",zCmd,0)!=0 ) zExe = ".exe";
-  if( file_is_absolute_path(zCmd) ){
-    return mprintf("%s%s", zCmd, zExe);
-  }
-  if( strchr(zCmd,'\\')!=0 && strchr(zCmd,'/')!=0 ){
-    int i;
-    Blob out = BLOB_INITIALIZER;
-    file_canonical_name(zCmd, &out, 0);
-    blob_append(&out, zExe, -1);
-    z = fossil_strdup(blob_str(&out));
-    blob_reset(&out);
-    for(i=0; z[i]; i++){ if( z[i]=='/' ) z[i] = '\\'; }
-    return z;
-  }
-  z = mprintf(".\\%s%s", zCmd, zExe);
-  if( file_isfile(z, ExtFILE) ){
-    int i;
-    Blob out = BLOB_INITIALIZER;
-    file_canonical_name(zCmd, &out, 0);
-    blob_append(&out, zExe, -1);
-    z = fossil_strdup(blob_str(&out));
-    blob_reset(&out);
-    for(i=0; z[i]; i++){ if( z[i]=='/' ) z[i] = '\\'; }
-    return z;
-  }
-  fossil_free(z);
-  zPath = fossil_getenv("PATH");
-  while( zPath && zPath[0] ){
-    int n;
-    char *zColon;
-    zColon = strchr(zPath, ';');
-    n = zColon ? (int)(zColon-zPath) : (int)strlen(zPath);
-    while( n>0 && zPath[n-1]=='\\' ){ n--; }
-    z = mprintf("%.*s\\%s%s", n, zPath, zCmd, zExe);
-    if( file_isfile(z, ExtFILE) ){
-      return z;
-    }
-    fossil_free(z);
-    if( zColon==0 ) break;
-    zPath = zColon+1;
-  }
-  return fossil_strdup(zCmd);
+  wchar_t buf[MAX_PATH];
+  GetModuleFileNameW(NULL, buf, MAX_PATH);
+  return buf;
 #else
   char *zPath;
   char *z;
   if( zCmd[0]=='/' ){
     return fossil_strdup(zCmd);

(7) By jshoyer on 2022-03-04 01:30:41 in reply to 6.1 [link] [source]

Indeed, when I apply that patch and run fossil diff -by I get what looks like exactly the same issue: a big block on the right-hand side gets duplicated many many times, truncated at the start by a few more characters each time in at least some of the copies.

Whereas when I run fossil diff -tk the diff looks right.

(8) By Martin Gagnon (mgagnon) on 2022-03-04 01:32:47 in reply to 6.1 [source]

With more experiments, I can reproduce only on my MacBook Pro (macOS Monterey 12.2.1 x86_64) with many recent version of fossil. Including latest 2.18 release from fossil webpage and latest trunk compiled by myself.

Any diff webpage generation will show the problem. (running fossil ui, fossil diff -by, fossil diff --webpage, etc...).

If the diff is very small (e.g. 2 o 3 line changes) will be OK. If the diff is a bit bigger (doesn't need to be big, 20'ish lines is enough, problem appear. In that case, the beginning of the diff is OK, and after a few lines, it start to break.

(9) By Martin Gagnon (mgagnon) on 2022-03-04 02:49:05 in reply to 6.1 [link] [source]

New development, compiling with -O0 make the problem disappear.

I hope I'll find that code that trigger this compiler bug (or may be an undefined behaviour ?)

Compiler version: Apple clang version 13.0.0 (clang-1300.0.29.30)

(10) By Martin Gagnon (mgagnon) on 2022-03-04 04:15:44 in reply to 6.1 [link] [source]

Just did a bisect session, and found out the the problem appears on this checkin: b0511022724d7bf0

It's progressing, trying to find the exact line now.

(11) By Martin Gagnon (mgagnon) on 2022-03-06 19:30:32 in reply to 6.1 [link] [source]

New developments:

  • I tried on a new M1 (arm64) MacBook on a freshly created user.

    • With native version 2.18 pre-compiled binary from fossil-scm download page. [No problem]
    • Run the x86_64 binary on same machine (works because of apple x86_64 binary emulation) [same problem happens]
  • Mark propose a patch that "backout" the checkin b0511022724d7bf0 on the latest trunk version ad744440dc89d439 and this also vanish the problem. (with standard ./configure)

  • compiling after using "./configure --with-sanitizer=null,undefined" make the problem disappear.

    • this will add the -fsanitize=null,undefined option to the compiler. I'm not sure what it really does.
    • as mention previously, compiling with no optimization (-O0), by using --no-opt or --fossil-debug with the configure script, also make the problem disappear.

Also, Mark have similar setup as me, same version of MacOS on a MacBookPro x64_64 like me, and he cannot reproduce the problem.

Getting closer, but I still cannot explain what's going on.

(12) By jshoyer on 2022-04-01 23:31:06 in reply to 11 [link] [source]

I can no longer reproduce the issue that I reported by building from either trunk (aa2066b5d2) or check-in b0511022724d7bf0. I have not had the issue since updating macOS to 12.2.1. (I did not immediately test after updating to 12.2.1, but did before and after updating to 12.3, and to 12.3.1 just now.) I hope the issue going away was related to the OS update, and that the issue will stay gone (e.g. if a system library or compiler bug was corrected).

If any pre-release Mac x64 binaries are put on the website in the lead-up to fossil 2.19 I'll test whether they suffer from the same issue.