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: Allow selectors in imported jinja templates #739

Merged

Conversation

stuarteberg
Copy link
Contributor

As tangentially mentioned in #728, it would be nice if selectors (e.g. # [not win]) were respected in not just meta.yaml itself, but also in any jinja templates that it happens to import. This PR implements support for just that, with a simple test.

@msarahan
Copy link
Contributor

This is sort of duplication of functionality to me - I thought the point of jinja2 stuff was at least partially to obviate selectors and replace them with something more flexible. Why have both of these? Are selectors just easier sometimes that the equivalent template expression?

@stuarteberg
Copy link
Contributor Author

Why have both of these?

In meta.yaml we already support both.

jinja is more powerful, but for historical reasons, I doubt selectors will be removed any time soon (right)? Since meta.yaml supports both, it seems reasonable to me if all code that meta.yaml might import also supports both.

Consider this contrived example. Suppose the user wants to tag each package with the user name of the person who made it, using the USER environment variable on unix. On Windows, she just tags the package with the generic string winuser:

{% set user_id = USER %} # [unix]
{% set user_id = "winuser" %} # [win]

package:
  name: mypackage
  version: 1.0

build:
  number: 1
  string: {{user_id}}_{{PKG_BUILDNUM}}

OK, that works just fine. But if she wants to do this in several packages, perhaps she'll want to define user_id in a separate file:

# meta.yaml
{% from 'variables.jinja' import user_id %}
package:
  name: mypackage
  version: 1.0

build:
  number: 1
  string: {{user_id}}_{{PKG_BUILDNUM}}
# variables.jinja
{% set user_id = USER %} # [unix]
{% set user_id = "winuser" %} # [win]

If she naively copies-and-pastes the lines from meta.yaml into variables.jinja, the selectors won't be used, and she'll end up with different results.

If she notices that this happened -- and understands why it happened -- she can fix it using the appropriate jinja logic instead:

# variables.jinja
{% if unix %}
  {% set user_id = USER %}
{% else %}
  {% set user_id = "winuser" %}
{% endif %}

That's a little uglier, but still not terrible. I think the bigger problem is that copy-and-paste from meta.yaml into an external file doesn't work like you'd expect.

@msarahan
Copy link
Contributor

Yes, I see your point. I thought jinja had nicer inline if statements. This is fine with me. @groutr @kalefranz, can either of you OK this?

@jankatins
Copy link

Isn't there a way to make the selectors available in the jinja context, similar to what now is done with load_setuptools?

@stuarteberg
Copy link
Contributor Author

Isn't there a way to make the selectors available in the jinja context, similar to what now is done with load_setuptools?

I think you're referring to the fact that certain variables are allowed in selectors, e.g. win, unix, py27, etc. Conveniently, those variables are already available in jinja templates, too. So, this is already allowed:

build:
  string: py{{py}}_npy{{np}}

This PR doesn't change that, but it also respects the removal of code with a selector (e.g. # [unix]), even if that line of code happens to reside in an imported jinja template instead of meta.yaml itself.

@stuarteberg stuarteberg force-pushed the selectors-for-imported-templates branch from 24a1dd6 to 796175a Compare January 26, 2016 22:32
@jakirkham
Copy link
Member

Seems like a simple and practical change. Any chance we can get this in? It seems to have sat here for awhile. Is there something blocking it that I don't understand?

@msarahan msarahan merged commit 3f52ada into conda:master May 5, 2016
@msarahan
Copy link
Contributor

msarahan commented May 5, 2016

Thanks @stuarteberg

@jakirkham
Copy link
Member

Thanks @msarahan.

@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.

4 participants