danyspin97 / rinstall

Declarative install for programs
GNU General Public License v3.0
28 stars 2 forks source link

rinstall tries to overwrite directories with files in some cases #6

Closed 0x5c closed 12 months ago

0x5c commented 1 year ago

The cause appears to be when paths don't end in a slash when trying to compute the destination path of files.

This can be seen in numerous places, including the default bindir path for users $HOME/.local/bin, along with any completion file.

Trying to use a freshly built rinstall to install itself:

$ cargo run --release -- install
>>> Package rinstall
WARNING: file /home/USER/.local/bin already exists, add --force to overwrite it
Would install target/release/rinstall -> /home/USER/.local/bin
WARNING: file /home/USER/.local/share/bash-completion already exists, add --force to overwrite it
Would install target/release/completions/rinstall.bash -> /home/USER/.local/share/bash-completion
WARNING: file /home/USER/.local/share/elvish/lib already exists, add --force to overwrite it
Would install target/release/completions/rinstall.elv -> /home/USER/.local/share/elvish/lib
Would install LICENSE.md -> /home/USER/.local/share/licenses/rinstall/LICENSE.md
Would install pkginfo -> /home/USER/.local/share/rinstall/rinstall.pkg

After adding a slash to all completion paths in package.rs and using --bindir $HOME/.local/bin/, the computed paths make sense

$ cargo run --release -- install --bindir $HOME/.local/bin/
>>> Package rinstall
Would install target/release/rinstall -> /home/USER/.local/bin/rinstall
Would install target/release/completions/rinstall.bash -> /home/USER/.local/share/bash-completion/rinstall.bash
Would install target/release/completions/rinstall.elv -> /home/USER/.local/share/elvish/lib/rinstall.elv
Would install LICENSE.md -> /home/USER/.local/share/licenses/rinstall/LICENSE.md
Would install pkginfo -> /home/USER/.local/share/rinstall/rinstall.pkg

Simply ensuring that all directory paths end with / seems to 'fix' the bug, but it almost feels like the real problem might be in how the file paths are computed.

danyspin97 commented 1 year ago

Yea, this is definitely a bug. Adding / at the end of directories in install.yml is mandatory but it definitely shouldn't be when setting directories in the command line. I think this is an issue due to how the files/directories are differentiated in the install.yml.

0x5c commented 1 year ago

iirc in that case the paths in package.rs are also to blame for not having that trailing slash (there was no install.yml in my testing at the time)

danyspin97 commented 1 year ago

I'll provide a fix this week, thanks for the report @0x5c :)