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

Metadata: Track undefined jinja names and decide whether to download source #964

Merged
merged 4 commits into from
May 22, 2016

Conversation

stuarteberg
Copy link
Contributor

@stuarteberg stuarteberg commented May 19, 2016

Background:
Some of the functions that decide whether or not to download the source before jinja rendering changed recently. (For example, 1a7ebca / cc6a3f9)

The function parse_or_try_download() decides whether the source is needed before the final parse. Previously, the decision was made based on the contents of the package/version field. As it turns out, this is not a sufficient condition, because it fails for a recipe like this:

package:
  name: foo
  version: 1.0

source:
  git_url: http://github.com/foo/foo

build:
  string: g{{ GIT_FULL_HASH[:7] }}
$ conda build foo
Error: Failed to render jinja template in foo/meta.yaml:
'GIT_FULL_HASH' is undefined

Solution:
During the first parse, we permit undefined jinja variables using a special class named UndefinedNeverFail. I tweaked that class so that it records the names of the undefined variables. These names are stored in Metadata.undefined_jinja_vars. Now, during parse_or_try_download(), we can just check the names of the variables that remain undefined. If any GIT_ variables were undefined, we need to download the source.

Note: This PR looks big, but that's just because the first commit moves some lines of code around (without changing them). I recommend reviewing these commits individually, not as a combined diff.

cc @msarahan

@stuarteberg stuarteberg force-pushed the remember-undefined-jinja-names branch from d560f43 to a43242b Compare May 20, 2016 00:18
@stuarteberg
Copy link
Contributor Author

FWIW, I fixed the flake8 error and now Travis is happy. Appveyor is giving some weird error about a bad zip file, but as far as I can tell that's totally unrelated to the changes in this PR.

@msarahan
Copy link
Contributor

LGTM. Only nitpick I'd ask is to change the name of the test folder to something more descriptive. Perhaps something like ...jinja2_build_str_template_only

@stuarteberg stuarteberg force-pushed the remember-undefined-jinja-names branch from a43242b to a62add3 Compare May 22, 2016 22:26
@stuarteberg
Copy link
Contributor Author

I'd ask is to change the name of the test folder

No prob; done.

@msarahan msarahan merged commit 1f94001 into conda:master May 22, 2016
@msarahan
Copy link
Contributor

Thanks.@stuarteberg

@stuarteberg
Copy link
Contributor Author

Crap, now I have 2 more commits I'd like to push. (Both are very minor.) I'll submit a follow-up PR.

@stuarteberg stuarteberg mentioned this pull request May 23, 2016
@github-actions github-actions bot added the locked [bot] locked due to inactivity label May 16, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
locked [bot] locked due to inactivity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants