mnurzia / even-better-ls

LS + Icons + Formatting
486 stars 31 forks source link

Using ls.c from coreutils 8.2 #27

Open Noammac opened 6 years ago

Noammac commented 6 years ago

The current version of coreutils is 8.29, but the version of ls.c provided in this repo is from 8 months ago - coreutils 8.2. This could potentially cause a lot of problems and potentially make the project uncompilable and buggy. Trying to port the project to a newer coreutils version (like I tried to do in my fork) introduces a plethora of bugs related to element alignment and filename escaping.

andretf commented 6 years ago

It actually gave me error when compiling patched ls.c, please see screenshot below. AFAIK coreutils 8.2 is from 2009, it has a lot of code since then. I patched 8.29 it myself and it went great..

afigueiredo andre-samsung -dev-vendor-even-better-ls-coreutils-8 29_002

Noammac commented 6 years ago

I actually managed to patch coreutils-8.28 (look at my fork) using comparative diffs but it had problems with text placement that I couldn't fix no matter what I tried.

CaptainTux commented 6 years ago

hello, i am getting the same error as in the screenshot above. i have tried to compile with the latest coreutils-2.9 on arch labs, but i cannot get it to work. any help is very much appreciated.

Noammac commented 6 years ago

Apparently, the install script downloads coreutils-8.2 but still uses ls.c from 8.2 (see #29). This is more of a problem than we thought.

Sebastiaan76 commented 6 years ago

Same problem here. Now I don't have a working ls! Definitely very broken.

raabf commented 6 years ago

Would it not be better to remove the entire ls.c file from the repository and just provide a .patch file for the changes of the original ls.c? The changes are really minimal (just 4 lines) and that would make this repo mostly independent from the specific coreutils version.

Noammac commented 6 years ago

Oh, it would be more efficient but won't make it entirely dependent from coreutils. It would still need to be updated constantly (though it would be easier to adapt it to newer versions manually).

I have done such a thing in the past with mixed results, but I do not have the original files I generated. I can attempt at doing it again, but I fear it will still come up with a mixed success.

On Sat, 19 May 2018, 21:52 Fabian Raab, notifications@github.com wrote:

Would it not be better to remove the entire ls.c file from the repository and just provide a .patch file for the changes of the original ls.c? The changes are really minimal (just 4 lines) and that would make this repo mostly independent from the specific coreutils version.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/illinoisjackson/even-better-ls/issues/27#issuecomment-390425166, or mute the thread https://github.com/notifications/unsubscribe-auth/ASpZq9-1s7AFuAznMcFso69aHEE5DRfKks5t0GnpgaJpZM4RopDJ .

raabf commented 6 years ago

Ah, I understant, I did not thought that there are so often so big changes in ls.c

Noammac commented 6 years ago

It's alright. The problem is that a lot of changes go on in the coreutils repository until it's deemed ready for a new release, so you can expect some changes in the structure and length of the code. Combine that with the fact that the patch format is dependant on all the lines of code staying in place, and now you have a problem.

On Sun, 20 May 2018, 20:04 Fabian Raab, notifications@github.com wrote:

Ah, I understant, I did not thought that there are so often so big changes in ls.c …

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/illinoisjackson/even-better-ls/issues/27#issuecomment-390496374, or mute the thread https://github.com/notifications/unsubscribe-auth/ASpZq4npjGDT1s0CVBR0SsIgtWu1NQoFks5t0aI3gaJpZM4RopDJ .

ev0rtex commented 6 years ago

In case it helps anyone else I encountered this problem and just made my own patch for coreutils-8.29 that should apply pretty cleanly (the effective changes are only a couple lines but I fixed indentation as well):

ls.patch:

--- src/ls.c    2017-12-10 18:12:46.000000000 -0700
+++ src/ls.c    2018-07-01 01:14:31.133443086 -0600
@@ -4753,21 +4753,15 @@
         }
     }

-  /* Check the file's suffix only if still classified as C_FILE.  */
-  ext = NULL;
-  if (type == C_FILE)
+  /* Test if NAME has a recognized suffix.  */
+  len = strlen (name);
+  name += len;     /* Pointer to final \0.  */
+  for (ext = color_ext_list; ext != NULL; ext = ext->next)
     {
-      /* Test if NAME has a recognized suffix.  */
-
-      len = strlen (name);
-      name += len;     /* Pointer to final \0.  */
-      for (ext = color_ext_list; ext != NULL; ext = ext->next)
-        {
-          if (ext->ext.len <= len
-              && STREQ_LEN (name - ext->ext.len, ext->ext.string,
-                            ext->ext.len))
-            break;
-        }
+      if (ext->ext.len <= len
+          && STREQ_LEN (name - ext->ext.len, ext->ext.string,
+                        ext->ext.len))
+        break;
     }

   /* Adjust the color for orphaned symlinks.  */
@@ -4826,6 +4820,9 @@
   if (print_scontext)
     len += 1 + (format == with_commas ? strlen (f->scontext) : scontext_width);

+  if (print_with_color)
+   len += 2;
+
   len += quote_name_width (f->name, filename_quoting_options, f->quoted);

   if (indicator_style != none)

...and then I also kinda tweaked the install script to only install from the local files + patch

install.sh:

COREUTILS_VER=8.29

#
# Copy over the colors generator
chmod 755 ls_colors_generator.py
sudo cp ls_colors_generator.py /usr/bin/ls_colors_generator

#
# Cleanup
[ -d "coreutils-${COREUTILS_VER}" ] && \
    rm -rf coreutils-${COREUTILS_VER}
[ -f "coreutils-${COREUTILS_VER}.tar.xz" ]  && \
    rm -f coreutils-${COREUTILS_VER}.tar.xz

#
# Download coreutils and patch
wget https://ftp.gnu.org/gnu/coreutils/coreutils-${COREUTILS_VER}.tar.xz
tar -xf coreutils-${COREUTILS_VER}.tar.xz
rm coreutils-${COREUTILS_VER}.tar.xz
cd coreutils-${COREUTILS_VER}
patch -p0 < ../ls.patch

#
# Compile and copy binaries
./configure
make
for bin in ls dir vdir; do
    echo "Copying ${bin} to /usr/bin/${bin}-i"
    sudo cp src/${bin} /usr/bin/${bin}-i
done

If any of this is useful I'd be happy to do a PR

xxb1s commented 5 years ago

@ev0rtex Bro!! Your solution work really great! thanks!!