abcxyz / abc

Apache License 2.0
12 stars 3 forks source link

Fix misguided semantics in local downloader #459

Closed drevell closed 8 months ago

drevell commented 8 months ago

Background: when installing a template from a local directory, we call it "canonical" if the template source directory and the destination directory are in the same git repo. When it's canonical, that means that relative path between the template source directory and the destinaton directory will be the same for other users that check out the same git repo. This means that future upgrades can be seamless, because we know where to get the latest template version from (e.g. "../../my_template").

When I implemented the code in LocalDownloader to detect whether a template installation was "canonical" or not, I did it wrong. I conflated the template directory and the destination directory. There are actually three directories of interest in the LocalDownloader, not just two:

  1. the source (where the template files are stored that we want to install)
  2. the template directory (a temporary copy of the contents of the source)
  3. the destination directory (where the user is rendering the template to).

The correct way to check canonical-ness is to compare (1) and (3). I was comparing (1) and (2) before.

This never affected users because all parts of the upgrade logic, including the canonical-ness calculation, are behind a hidden flag that defaults to false.

This unblocks more work on the template upgrade feature and was discovered while implementing that.

I filed #458 to reconsider whether we even need a template temp dir (directory number 2 above) in the case where we're installing from a local directory.