dnephin / dobi

A build automation tool for Docker applications
https://dnephin.github.io/dobi/
Apache License 2.0
311 stars 36 forks source link

Bug: PR #184 breaks sources and artifacts #200

Closed siredmar closed 3 years ago

siredmar commented 4 years ago

This is a bug report. I tried using V0.14.0 but wasn't able to run my previous working dobi scripts. I narrowed it down to PR #184. This commit breaks the sources/artifact system.

The build fails with a message like this:

[WARN] [job: build-xyz] /usr/local/bin/build.sh -> build/<artifact-path> 
Failed to get artifact last modified: Rel: can't make build/<artifact-path> relative to <basepath>
siredmar commented 4 years ago

@tduffield @dnephin this is to inform you

siredmar commented 4 years ago

@dnephin @tduffield I made a minimalist example on how to see this bug: https://github.com/siredmar/dobi_sources_artifacts_broken

sudo curl -L https://github.com/dnephin/dobi/releases/download/v0.13.0/dobi-linux -o /usr/local/bin/dobi.13
sudo curl -L https://github.com/dnephin/dobi/releases/download/v0.14.0/dobi-linux -o /usr/local/bin/dobi.14
sudo chmod +x /usr/local/bin/dobi.13 /usr/local/bin/dobi.14
git clone https://github.com/siredmar/dobi_sources_artifacts_broken
cd dobi_sources_artifacts_broken
dobi.13 install
dobi.14 install

Please have a look at the install target and the different source/artifacts configurations to see what is going wrong.

siredmar commented 4 years ago

I investigated a little bit and basically the issue is that filepath.Rel() in directory.go tries to create the relative path for the rootpath and the filepath. filepath.Rel() however checks if both arguments are absolute paths (e.g. /path/to/something). If one or both arguments are not absolute paths (e.g. build/a.out) this breaks.

So, we have a few options here:

  1. Revert the commit, as is cannot find any discussion about that topic and the need for the change at all.
  2. We can enable config variables in sources/artifact values, and let the user put {fs.projectdir} in his values. This must be documented very well, because this breaks if not used correctly (like it does right now with 0.14.0).
  3. If paths in sources/artifact values are relative, put {env.cwd} automatically in front of the values.
dnephin commented 3 years ago

Fixed by #209