thoughtbot / rcm

rc file (dotfile) management
https://thoughtbot.github.io/rcm/rcm.7.html
BSD 3-Clause "New" or "Revised" License
3.13k stars 136 forks source link

EXCLUDES not working as expected #11

Closed pablox-cl closed 7 years ago

pablox-cl commented 11 years ago

I have the following files in my ~/.dotfiles directory:

config/systemd/user
config/xfce4
systemd

I only want to exclude config/systemd/*, so I've tried the following:

EXCLUDES="config/systemd" 
$ lsrc -v 
/home/pablo/.config/systemd/user/default.target:/home/pablo/.dotfiles/config/systemd/user/default.target
/home/pablo/.config/systemd/user/dropbox@.service:/home/pablo/.dotfiles/config/systemd/user/dropbox@.service
/home/pablo/.config/systemd/user/empty.target:/home/pablo/.dotfiles/config/systemd/user/empty.target
/home/pablo/.config/systemd/user/gnome-do@.service:/home/pablo/.dotfiles/config/systemd/user/gnome-do@.service
/home/pablo/.config/systemd/user/numlockx@.service:/home/pablo/.dotfiles/config/systemd/user/numlockx@.service
[ ... a lot of output removed .... ]
/home/pablo/.systemd:/home/pablo/.dotfiles/systemd
EXCLUDES="systemd" # or "*:systemd"
$ lsrc -v 
/home/pablo/.config/xfce4/......
[ ... a lot of output removed .... ]

Following the instructions for the lsrc man page, I followed the idea of the thoughbot-dotfiles example:

EXCLUDES="config:systemd" # or "config:*systemd*"
$ lsrc -v 
/home/pablo/.config/systemd/user/default.target:/home/pablo/.dotfiles/config/systemd/user/default.target
/home/pablo/.config/systemd/user/dropbox@.service:/home/pablo/.dotfiles/config/systemd/user/dropbox@.service
/home/pablo/.config/systemd/user/empty.target:/home/pablo/.dotfiles/config/systemd/user/empty.target
/home/pablo/.config/systemd/user/gnome-do@.service:/home/pablo/.dotfiles/config/systemd/user/gnome-do@.service
/home/pablo/.config/systemd/user/numlockx@.service:/home/pablo/.dotfiles/config/systemd/user/numlockx@.service
[ ... a lot of output removed .... ]
/home/pablo/.systemd:/home/pablo/.dotfiles/systemd

Same thing... am I misunderstanding how it works?

pbrisbin commented 10 years ago

The excludes pattern is (<source-glob>:)<file-glob>. And I believe file-glob is attempted against the full relative path within the source if it contains a slash, otherwise just the basename.

Your first example says to match "config/system" in any source. The files "config/systemd/user/..." do not match this since the slash makes them full-path checks.

Your second examples both say to match "systemd" in any source. Since this is a basename match, it filters out "config/systemd/*" and "systemd".

Your third example says to match "systemd" and "*systemd" both in the "config" source. This won't work because your source is ".dotfiles", not "config".

I would try this:

EXCLUDES="config/systemd/*"

Interestingly, this is the exact thing you stated in English, but the one permutation you didn't attempt in code :)

pablox-cl commented 10 years ago

I'm pretty sure I tried that O.o, but oh well. Everything can happen. I'm gonna test it asap.

pablox-cl commented 10 years ago

I actually test it and doesn't work. It just ignore that particular EXCLUDE directive:

EXCLUDES="hola/chao/*"

$ lsrc -v
recurring on /home/pablo/.hola/chao
/home/pablo/.hola/chao/archivo:/home/pablo/.dotfiles/hola/chao/archivo
/home/pablo/.hola/chao/asdf:/home/pablo/.dotfiles/hola/chao/asdf
/home/pablo/.hola/chao/file:/home/pablo/.dotfiles/hola/chao/file

With debug on:

handling the file archivo
handle_file archivo /home/pablo/.hola/chao /home/pablo/.dotfiles hola/chao 1 README.md systemd zsh.d zprezto dircolors hola/chao/archivo hola/chao/asdf hola/chao/file
is_excluded archivo README.md systemd zsh.d zprezto dircolors hola/chao/archivo hola/chao/asdf hola/chao/file
mike-burns commented 10 years ago

The problem is in handle_file, where it calls is_excluded. It passes the basename instead of the path.

I tried changing it to pass the path:

elif is_excluded "$dotfiles_subdir/$file" "$exclude_file_globs" "$include_file_globs"; then

but then it broke for the case when you don't pass it a glob (in the example you give, it breaks if you pass -x systemd).

While working on this I discovered that the glob can get resolved too easily, and e.g. -x *systemd* will turn into -x systemd as soon as the glob matches something.

@pbrisbin you're a sh quoting expert. Any advice here?

(I just found myself thinking, "if I converted all the strings to binary my life would be easier," and then realized just how crazy that sounds.)

pbrisbin commented 10 years ago

I'm not sure this is a quoting issue. It's true that the potential globs should be quoted while being passed around and can't be quoted in the case evaluation, but that's unrelated, I think.

What about the following logic for is_excluded:

Would that cover all cases?

mike-burns commented 10 years ago

Yeah, @pbrisbin , that might work.

I'd like someone besides me to take a crack at this; I'm having trouble thinking of a great solution.

mike-burns commented 10 years ago

I don't know how to pass a glob around without it being expanded:

~% ls -d *vim*
vimulator/
~% cat echo.c 
#include <stdio.h>

int main(int argc, char *argv[]) {
  int i;

  for (i = 1; i < argc; i++)
    printf("%s", argv[i]);

  printf("\n");

  return 0;
}
~% gcc -o e echo.c
~% ./e hello
hello
~% ./e *vim*
vimulator
pbrisbin commented 10 years ago

You just have to quote it.

$ touch foo bar baz
$ for g in 'f*' 'b*'; do # strings
>   ls "$g" # still string
>   ls $g   # expandable glob
> done
ls: cannot access f*: No such file or directory
foo
ls: cannot access b*: No such file or directory
bar  baz
mike-burns commented 10 years ago

OK, that resolves that a bit, but even when I quote things lsrc(1) expands them:

~% lsrc -vvv -x '*vim*' 2>&1 | head
TAGS: debian development email git laptop thoughtbot gmail notmuch debian-email gpg
DOTFILES_DIRS: 
DOTFILES_DIRS:  /home/mike/.dotfiles
COPY_ALWAYS: 
SYMLINK_DIRS: 
dotfiles_dir_excludes /home/mike/.dotfiles
  with excludes:  *vim*
exclude_file_globs: vimulator  #### THIS
dotfiles_dir_excludes /home/mike/.dotfiles
  with excludes: 
pbrisbin commented 10 years ago

It's always quoting. I'm going to coin Pat's theorem:

In shell, any unintentional lack of quotation is broken. You just don't know it yet.

And the corollary:

Most intentional cases are broken too.

here:

for exclude in $excludes; do # globs expand
  # ...

There may be a set option to prevent globbing (which you'll have to turn on here, then immediately off again for the case). I don't know it though, or if it's POSIX. Also, this doesn't fix the fundamental problem, the lack of quotation would most likely still be broken in some way. See Pat's theorem ;)

mike-burns commented 10 years ago

How can I iterate over a space-separated list of globs? Where do I put quotes to make this happen?

mike-burns commented 10 years ago

I've opened #54. It doesn't fix this problem at all, but it's a decent start.

pbrisbin commented 10 years ago

(Haven't looked at it, stuck on train but...)

How about concatting the patterns with "|" at arg parsing, then doing a single glob comparison in the case?

This makes the full-path-vs-basename problem harder though :/

BTW, for x in $thing is always broken and should be avoided, regardless of quoting. On Mar 6, 2014 9:35 AM, "Mike Burns" notifications@github.com wrote:

I've opened #54 https://github.com/thoughtbot/rcm/pull/54. It doesn't fix this problem at all, but it's a decent start.

Reply to this email directly or view it on GitHubhttps://github.com/thoughtbot/rcm/issues/11#issuecomment-36893061 .

pbrisbin commented 10 years ago

If we have to keep it in a space separated list, something like this may work:

matches_patterns() {
  local file="$1" patterns="$2"

  echo "$patterns" | sed 's/ /\n/g' | while read pattern; do
    # treat as glob, file, base, etc here...

    case "$file" of
      $pattern) return 0 ;;
    esac
  done

  return 1
}
mike-burns commented 10 years ago

Yeah, set -f disables path expansion.

$ foo="a b c *vim*"
$ for z in $foo; do
> echo $z
> done
a
b
c
vimulator
$ set -f
$ for z in $foo; do
> echo $z
> done
a
b
c
*vim*
mike-burns commented 10 years ago

I've opened #55, too, for discussion on that topic.

mike-burns commented 10 years ago

I've opened #58. I'd like to push #11 to milestone 1.3.0, merge #58, and release 1.2.1. It seems that #11 will take longer than expected.

@PaBLoX-CL @pbrisbin any objections to this?

pablox-cl commented 10 years ago

No. It's fine by me. Anyway, I believe for most use cases, the actual implementation works fine. The edge cases or when you want to specify a particular file inside a subdirectory it's when things go eerie.


I know the idea is to keep this project as POSIX as possible, but it seems a POSIX compatible shell wasn't built for this task. Why not using perl, sed or awk to accomplish this?

mike-burns commented 10 years ago

The idea is to keep the project as able-to-run-on-any-machine-out-of-the-box as possible. My targets are (in order) FreeBSD, GNU, OS X, Solaris. This leaves us with POSIX sh and POSIX awk (sed won't suffice).

That said, I'd be willing to entertain the idea of a binary+VM packaged together.

pablox-cl commented 10 years ago

And what about perl?

I guess my Archlinux distro falls in the GNU category right?

mike-burns commented 10 years ago

Perl is far from standard. FreeBSD is the first example that comes to mind.

The only Linux distros I know of that are not GNU are Android, Busybox, and UNG/Linux.

mike-burns commented 10 years ago

This was closed because GitHub is clever.

mike-burns commented 10 years ago

Short of the Haskell rewrite: we could apply the globs after-the-fact in lsrc(1). So collect all files, then go through the list and exclude any that match the appropriate globs.

mike-burns commented 8 years ago

Captured in a test on the exclude-globs branch: https://github.com/thoughtbot/rcm/blob/exclude-globs/test/lsrc-excludes.t#L22-L32

mike-burns commented 7 years ago

This might be fixed by #192.