deadc0de6 / dotdrop

Save your dotfiles once, deploy them everywhere
https://dotdrop.readthedocs.io
GNU General Public License v3.0
1.78k stars 105 forks source link

[bug] Only first `import_variables` taken when multiple are given #380

Closed adnan360 closed 1 year ago

adnan360 commented 1 year ago

Dotdrop version (and git commit if run from source): v1.12.9 Using dotdrop: from apt on Devuan Daedalus

I'm on Devuan Daedalus (should be similar to Debian testing) and I installed dotdrop from apt repo (v1.12.9).

$ apt search dotdrop
Sorting... Done
Full Text Search... Done
dotdrop/testing,now 1.12.9-1 all [installed]
  save your dotfiles once deploy them everywhere
$ dotdrop --version
1.12.9

Describe the bug

I use import_variables to mention multiple files with secondary files :optional appended at the end. Currently it just reads values from the first one and ignores the rest.

I want to have a default set of variables in a file. And when optionally, user creates a copy of the file I want that file to override the values from the default variables file. I want the override file to work even if only a subset of variables (not all) are mentioned for override.

Summary:

Steps to Reproduce

Steps to reproduce the behavior:

Here is a test repo with files below for easy testing.

config.yaml

config:
  backup: false
  banner: true
  create: true
  dotpath: ./
  ignoreempty: false
  keepdot: false
  longkey: false
  showdiff: false
  workdir: ./
  import_variables:
    - custom.default.yaml
    - custom.yaml:optional
dotfiles:
  test1:
    dst: output
    src: src
profiles:
  default:
    dotfiles:
      - test1

custom.default.yaml

variables:
  string1: 'unoverriden value'
  string2: 'default value'

custom.yaml

variables:
  string2: 'override value from custom.yaml'

src/somefile.txt

String1: {{@@ string1 @@}}
String2: {{@@ string2 @@}}

Then run:

dotdrop install -p default -f && cat output/somefile.txt

Expected behavior

$ dotdrop install -p default -f && cat output/somefile.txt
     _       _      _
  __| | ___ | |_ __| |_ __ ___  _ __
 / _` |/ _ \| __/ _` | '__/ _ \| '_ |
 \__,_|\___/ \__\__,_|_|  \___/| .__/  v1.12.9
                               |_|

    -> install ... to ...

1 dotfile(s) installed.
String1: unoverriden value
String2: override value from custom.yaml

Process all files on list, override variables as it goes through the list. Output should say override value from custom.yaml.

Current behavior

$ dotdrop install -p default -f && cat output/somefile.txt
     _       _      _
  __| | ___ | |_ __| |_ __ ___  _ __
 / _` |/ _ \| __/ _` | '__/ _ \| '_ |
 \__,_|\___/ \__\__,_|_|  \___/| .__/  v1.12.9
                               |_|

    -> install ... to ...

1 dotfile(s) installed.
String1: unoverriden value
String2: default value

Processes only the first file and ignores the rest. Tried removing :optional to see if it works. That even didn't work.

Additional information

The relevant part of the config file (given above)

Dotdrop's execution with the debug logs (--verbose)

$ dotdrop install -p default -f --verbose

Please check output here

Any additional information that would help reproduce the bug.

https://codeberg.org/adnan360/dotdrop-test

deadc0de6 commented 1 year ago

Hey @adnan360, thanks for reporting this and sorry you're having issues with dotdrop. It is indeed a bug and I'm working on fixing it in branch import-variables.

The fix in that branch should solve your issue and you can test it by using the latest dotdrop version from the branch (see this doc).

Once we can confirm it does fix your issue, I will release a new version of dotdrop. However be aware that the new version might not be immediately available in debian repo.

deadc0de6 commented 1 year ago

Based on your test example (thanks a lot for that btw), it seems the fix is working:

$ git rev-parse --abbrev-ref HEAD
import-variables
$ ./dotdrop.sh --cfg /tmp/dotdrop-test/config.yaml install -p default -f
    -> install /tmp/dotdrop-test/src/somefile.txt to /tmp/dotdrop-test/output/somefile.txt
$ cat /tmp/dotdrop-test/output/somefile.txt
String1: unoverriden value
String2: override value from custom.yaml
adnan360 commented 1 year ago

I'm so relieved. Thanks for looking into it.

I had this bug for over a year most probably. First I encountered it on a OpenBSD/FreeBSD install. So I thought it is probably BSD specific and applied some fix to a install script, added some note and moved on. Since now encountered it on a GNU+Linux install I thought this should be reported. It's nice to get a confirmation it is an actual bug.

I tried the branch you mentioned. For some reason it is not producing anything in output dir:

$ git clone --depth 1 -b import-variables https://github.com/deadc0de6/dotdrop.git
$ cd dotdrop
$ git rev-parse --abbrev-ref HEAD
import-variables
$ git log --oneline
8fa947a (grafted, HEAD -> import-variables, origin/import-variables) fix doc
$ ./dotdrop.sh --version
1.12.11
$ cd ../dotdrop-test
$ rm output/* &>/dev/null ; ../dotdrop/dotdrop.sh install -p default -f && cat output/somefile.txt
     _       _      _
  __| | ___ | |_ __| |_ __ ___  _ __
 / _` |/ _ \| __/ _` | '__/ _ \| '_ |
 \__,_|\___/ \__\__,_|_|  \___/| .__/  v1.12.11
                               |_|

[WARN] no dotfile to install for this profile ("default")

I'm probably testing it wrong. Another possibility is that Devuan did something so that it does not work. I noticed that it does not support installing anything with pip (shows a warning instead) and recommends to either use apt packages or build with venv.

deadc0de6 commented 1 year ago

I tried the branch you mentioned. For some reason it is not producing anything in output dir:

The dotdrop.sh script will pivot to its own directory and then load the config file. Since you didn't provide it explicitly, it just used the one provided in the dotdrop git tree (that one), which does not contain any dotfile.

You can make the example work by specifying the config file using an absolute path for example:

rm output/* &>/dev/null ; ../dotdrop/dotdrop.sh install -p default -f --cfg $(pwd)/config.yaml && cat output/somefile.txt
adnan360 commented 1 year ago

It worked, thanks.

$ rm output/* &>/dev/null ; ../dotdrop/dotdrop.sh install -p default -f --cfg $(pwd)/config.yaml && cat output/somefile.txt
     _       _      _
  __| | ___ | |_ __| |_ __ ___  _ __
 / _` |/ _ \| __/ _` | '__/ _ \| '_ |
 \__,_|\___/ \__\__,_|_|  \___/| .__/  v1.12.11
                               |_|

    -> install /home/.../dotdrop-test/src/somefile.txt to /home/.../dotdrop-test/output/somefile.txt

1 dotfile(s) installed.
String1: unoverriden value
String2: override value from custom.yaml

I tried with another file to see if it can override the values. It was successful too.

config.yaml

  import_variables:
    - custom.default.yaml
    - custom.yaml:optional
    - custom2.yaml:optional

custom2.yaml

variables:
  string2: 'override value from custom2.yaml'

output:

...
String1: unoverriden value
String2: override value from custom2.yaml

Feel free to merge this when ready.

deadc0de6 commented 1 year ago

@adnan360 dotdrop version 1.12.13 has been released and contains the bug fix. Thanks again for your help on this.