Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Namespaced templates don't work on windows #4394

Closed
bradrn opened this issue Nov 12, 2018 · 8 comments

Comments

Projects
None yet
2 participants
@bradrn
Copy link
Contributor

commented Nov 12, 2018

Steps to reproduce

Run stack new test-yesod yesodweb/simple.

Expected

Downloads the template at https://github.com/yesodweb/stack-templates/blob/master/simple.hsfiles.

Actual

Errors with That template doesn't exist:

> stack new test-yesod yesodweb/simple --verbose
Version 1.9.1, Git revision f9d0042c141660e1d38f797e1d426be4a99b2a3c (6168 commits) x86_64 hpack-0.31.0
2018-11-12 17:58:24.419878: [debug] Opening local template: "yesodweb\simple.hsfiles"
2018-11-12 17:58:24.419878: [info] Downloading template "yesodweb/simple" to create project "test-yesod" in test-yesod\ ...
2018-11-12 17:58:24.419878: [debug] Downloading /commercialhaskell/stack-templates/master/yesodweb%5Csimple.hsfiles
That template doesn't exist. Run `stack templates' to discover available templates.

The problem seems to be that stack is still looking in the https://github.com/commercialhaskell/stack-templates repo - but even when I change it to the URL it still doesn't work:

> stack new test-yesod https://github.com/yesodweb/stack-templates/simple.hsfiles -v
Version 1.9.1, Git revision f9d0042c141660e1d38f797e1d426be4a99b2a3c (6168 commits) x86_64 hpack-0.31.0
2018-11-12 18:02:00.203221: [info] Downloading template "https://github.com/yesodweb/stack-templates/simple.hsfiles" to create project "test-yesod" in test-yesod\ ...
2018-11-12 18:02:00.203221: [debug] Downloading /yesodweb/stack-templates/simple.hsfiles
That template doesn't exist. Run `stack templates' to discover available templates.

Stack version

> stack --version
Version 1.9.1, Git revision f9d0042c141660e1d38f797e1d426be4a99b2a3c (6168 commits) x86_64 hpack-0.31.0

(Also, I'm using Windows.)

Method of installation

Official binary, downloaded from stackage.org or fpcomplete's package repository

@dbaynard

This comment has been minimized.

Copy link
Contributor

commented Nov 12, 2018

Hello @bradrn

Thanks for raising the issue. Template handling has changed in 1.9.1 — I'll check the details and get back to you tomorrow.

@bradrn

This comment has been minimized.

Copy link
Contributor Author

commented Nov 13, 2018

@dbaynard I know template handling has changed in 1.9.1 — it's one of the new template features which I'm having trouble with.

@dbaynard

This comment has been minimized.

Copy link
Contributor

commented Nov 13, 2018

It works for me on 1.9.0.1 (the RC) — I've put the output below.

Version 1.9.0.1, Git revision 4dee68538c3f6305d9531f4ad997d8c15b7f4434 (6125 commits) x86_64 hpack-0.31.0
2018-11-13 22:06:28.871382: [debug] Opening local template: "yesodweb/simple.hsfiles"
2018-11-13 22:06:28.871816: [info] Downloading template "yesodweb/simple" to create project "test4394" in test4394/ ...
2018-11-13 22:06:28.871915: [debug] Downloading /yesodweb/stack-templates/master/simple.hsfiles
2018-11-13 22:06:29.153647: [debug] Opening local template: "/home/db/.stack/templates/yesodweb/simple.hsfiles"
2018-11-13 22:06:29.313226: [info] Looking for .cabal or package.yaml files to use to init the project.
2018-11-13 22:06:29.315047: [info] Using cabal packages:
2018-11-13 22:06:29.315141: [info] - test4394/

2018-11-13 22:06:29.315225: [debug] Running hpack on /home/db/Projects/stack-triage/test4394/package.yaml
2018-11-13 22:06:29.322892: [debug] hpack generated a modified version of /home/db/Projects/stack-triage/test4394/test4394.cabal
2018-11-13 22:06:29.328987: [debug] Downloading snapshot versions file from https://s3.amazonaws.com/haddock.stackage.org/snapshots.json
2018-11-13 22:06:30.209023: [debug] Done downloading and parsing snapshot versions file
2018-11-13 22:06:30.209320: [info] Selecting the best among 14 snapshots...

2018-11-13 22:06:30.209657: [debug] Decoding build plan from: /home/db/.stack/build-plan/lts-12.18.yaml
2018-11-13 22:06:30.209859: [debug] Trying to decode /home/db/.stack/build-plan-cache/lts-12.18.cache
2018-11-13 22:06:30.222219: [debug] Success decoding /home/db/.stack/build-plan-cache/lts-12.18.cache
2018-11-13 22:06:30.222596: [debug] Trying to decode /home/db/.stack/loaded-snapshot-cache/x86_64-linux/__snapshot_hints__/lts-12.18.cache
2018-11-13 22:06:30.271445: [debug] Success decoding /home/db/.stack/loaded-snapshot-cache/x86_64-linux/__snapshot_hints__/lts-12.18.cache
2018-11-13 22:06:30.272262: [info] * Matches lts-12.18
2018-11-13 22:06:30.272351: [info]
2018-11-13 22:06:30.272391: [info] Selected resolver: lts-12.18
2018-11-13 22:06:30.272849: [debug] Trying to decode /home/db/.stack/loaded-snapshot-cache/x86_64-linux/__snapshot_hints__/lts-12.18.cache
2018-11-13 22:06:30.317268: [debug] Success decoding /home/db/.stack/loaded-snapshot-cache/x86_64-linux/__snapshot_hints__/lts-12.18.cache
2018-11-13 22:06:30.317760: [info] Initialising configuration using resolver: lts-12.18
2018-11-13 22:06:30.317830: [info] Total number of user packages considered: 1
2018-11-13 22:06:30.317861: [info] Writing configuration to file: test4394/stack.yaml
2018-11-13 22:06:30.318646: [info] All done.

(Also, I'm using Windows.)

Edit: works for me on Linux.

@dbaynard

This comment has been minimized.

Copy link
Contributor

commented Nov 13, 2018

I think this is a Windows issue.

2018-11-12 17:58:24.419878: [debug] Opening local template: "yesodweb\simple.hsfiles"

Note the backslash, where you entered a solidus? This is probably overeager path conversion for Windows.

The relevant PR is #4103. I got as far as this, for now:

else do relDir <- parseRelDir (packageNameString project)

It's treating the namespaced template as a path.

@dbaynard dbaynard changed the title Templates don't work outside commercialhaskell/stack-templates Namespaced templates don't work on windows Nov 13, 2018

@bradrn

This comment has been minimized.

Copy link
Contributor Author

commented Nov 13, 2018

I'm not too sure that line in Stack.New is the source of the bug - if you look at it, it's dealing with the name of the project rather than the template. Meanwhile, I've discovered something interesting - if you prefix the template name with github:, everything suddenly starts to work! So the bug's still there, but at least there's a way around it now...

@bradrn

This comment has been minimized.

Copy link
Contributor Author

commented Nov 14, 2018

I think I've managed to find the bug. The issue is in these part of Stack.Types.TemplateName:

validParses prefix hsf orig =
-- NOTE: order is important
[ TemplateName prefix . RepoPath <$> parseRepoPath hsf
, TemplateName (T.pack orig) . UrlPath <$> (parseRequest orig *> Just orig)
, TemplateName prefix . AbsPath <$> parseAbsFile hsf
, TemplateName prefix . RelPath <$> parseRelFile hsf
]

-- | Parses a template path of the form @github:user/template@.
parseRepoPath :: String -> Maybe RepoTemplatePath
parseRepoPath s =
case T.splitOn ":" (T.pack s) of
["github" , rest] -> parseRepoPathWithService Github rest
["gitlab" , rest] -> parseRepoPathWithService Gitlab rest
["bitbucket" , rest] -> parseRepoPathWithService Bitbucket rest
_ -> Nothing

If the template name begins with github:, the template name will succeed in being parsed using parseRepoPath; if not, it will be parsed as a relative path using parseRelFile — which will of course convert / to \ on Windows. This causes problems with this exception handler, which downloads a template if it does not already exist locally. I think that the easiest solution would be to simply convert all the slashes back in that exception handler, but I'm not sure if that would work in all cases.

@dbaynard

This comment has been minimized.

Copy link
Contributor

commented Nov 14, 2018

I'm not too sure that line in Stack.New is the source of the bug

Thanks 👍 — and thanks for tracking down the bug itself, too.

I think that the easiest solution would be to simply convert all the slashes back in that exception handler, but I'm not sure if that would work in all cases.

Right, so when stack sees yesodweb/simple it doesn't know whether that means the local file yesodweb/simple.hsfiles or the remote repo github:yesodweb/simple. It might also need to be said explicitly in the code that github is the default service, from this perspective.

I don't like that a RelPath might not be a RelPath (I don't like when data types lie — this is a great example of fragility that comes from that). The right fix would be to ensure that a RelPath is actually a relative path and not a repo in disguise.

Would you be willing to work on a PR, @bradrn?

@bradrn

This comment has been minimized.

Copy link
Contributor Author

commented Nov 15, 2018

Right, so when stack sees yesodweb/simple it doesn't know whether that means the local file yesodweb/simple.hsfiles or the remote repo github:yesodweb/simple.

Yes.

It might also need to be said explicitly in the code that github is the default service, from this perspective.

This is already done:

stack/src/Stack/New.hs

Lines 305 to 307 in f9d0042

-- | The default service to use to download templates.
defaultRepoService :: RepoService
defaultRepoService = Github

I don't like that a RelPath might not be a RelPath (I don't like when data types lie — this is a great example of fragility that comes from that). The right fix would be to ensure that a RelPath is actually a relative path and not a repo in disguise.

Admittedly, there is a comment next to the constructor clarifying the name:

| RelPath (Path Rel File)
-- ^ a relative path on the filesystem, or relative to
-- the template repository

Would you be willing to work on a PR, @bradrn?

Yes, I would definitely be willing to work on a PR. Unfortunately, I'm not entirely sure how to fix this problem, as it is impossible to distinguish a RelPath from a RepoPath without doing IO to find if the file already exists. I'm thinking that the best approach here would be to add a Text field to the RelPath constructor, containing the argument originally passed. Do you think this would be a good idea?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.