Making of copy of the input template may be unnecessary when it's a local directory #458
Labels
enhancement
New feature or request
Comments
drevell
added a commit
that referenced
this issue
Feb 27, 2024
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) to (3). I was
comparing (1) to (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.
drevell
added a commit
that referenced
this issue
Feb 27, 2024
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.
|
I remembered why we don't do this: when abc create the template directory itself, it can guarantee that there are no symlinks. If there were symlinks, things could get complex. We could scan the template directory for symlinks and reject any that refer to a location that's outside the template directory. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Currently, we always create a temporary directory called the "template directory," and copy the template contents into that directory. This makes sense when the template is a remote (e.g. a github repo), but is perhaps unnecessary when the template is just stored in a local directory.
The LocalDownloader could perhaps be replaced by a no-op that just points to the existing template files that are already on the filesystem. Currently the LocalDownloader copies the files into the template directory (a temp dir) for no great reason.
Before we do this, we'll want to be sure that we never modify the template directory. Because it's OK to modify our private temp dir copy of the template, but it's not OK to modify any files outside of temp dirs that are owned by the user.
The text was updated successfully, but these errors were encountered: