Skip to content

Make embedded files be resolved by project root#266

Merged
snoyberg merged 4 commits intoyesodweb:masterfrom
SupercedeTech:templates-relative-to-project-root
Apr 25, 2022
Merged

Make embedded files be resolved by project root#266
snoyberg merged 4 commits intoyesodweb:masterfrom
SupercedeTech:templates-relative-to-project-root

Conversation

@ivb-supercede
Copy link
Copy Markdown
Contributor

Because file embedding runs IO in the Template Haskell Q monad, relative file paths are resolved by the compiler's working directory. Ordinarily, this is the root of the project being built - but if the compiler is e.g. building a dependency, it can also be different.

This change makes it so that template files are always embedded relative to the project that contains the code embedding them, rather than the working directory of the compiler - this means that dependencies in a multi-project codebase can embed files correctly.

Because file embedding runs IO in the Template Haskell `Q` monad,
relative file paths are resolved by the compiler's working directory.
Ordinarily, this is the root of the project being built - but if the
compiler is e.g. building a dependency, it can also be different.

This change makes it so that template files are always embedded relative
to the project that contains the code embedding them, rather than the
working directory of the compiler - this means that dependencies in a
multi-project codebase can embed files correctly.
Comment on lines +288 to +299
fp <- makeRelativeToProject rawFp
qRunIO (readUtf8FileString fp)

-- | Embed file's content, converting newlines
-- and track file via ghc dependencies, recompiling on changes
--
-- @since 2.0.19
readFileRecompileQ :: FilePath -> Q String
readFileRecompileQ fp = addDependentFile fp >> readFileQ fp
readFileRecompileQ rawFp = do
fp <- makeRelativeToProject rawFp
addDependentFile fp
qRunIO (readUtf8FileString fp)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't figure out a nice way to unify these. I didn't want to make readUtf8FileString run in Q, since it didn't seem concerned with being a TH function.

Copy link
Copy Markdown
Member

@snoyberg snoyberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Can you bump the minor version number and add a note in the changelog?

@ivb-supercede
Copy link
Copy Markdown
Contributor Author

Just noticed messages have the same issue, so I'll fix those too.

This is for the same motivation as the change to template resolution,
which is support for multi-project repositories.
Copy link
Copy Markdown
Member

@snoyberg snoyberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@snoyberg snoyberg merged commit dd717a9 into yesodweb:master Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants