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

Add lints that prevent jinja variable misuse. Add subcommand for bumping build number. #339

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions bioconda_utils/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -632,6 +632,16 @@ def pypi_check(recipe_folder, config, loglevel='info', packages='*', only_out_of
print('\t'.join(map(str, result)))


@arg('recipe', nargs="+", help='Path to recipes')
@arg('--loglevel', default='debug', help='Log level')
def bump_build_number(recipes, loglevel="info"):
"""
Bumps the build number in given recipes.
"""
utils.setup_logger('bioconda_utils', loglevel)
utils.bump_build_number(recipes)


def main():
argh.dispatch_commands([
build, dag, dependent, lint, duplicates,
Expand Down
77 changes: 77 additions & 0 deletions bioconda_utils/lint_functions.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import os
import re

import yaml
import pandas
import numpy as np

Expand Down Expand Up @@ -93,6 +94,77 @@ def lint_metas(recipe, metas, df, *args, **kwargs):
return lint_metas


JINJA_VAR_DEF = re.compile(r"{%.+?%}")
JINJA_VAR = re.compile(r"{{.*?}}")


def load_plain_yaml(recipe):
"""Load meta.yaml without applying any jinja templating."""
with open(os.path.join(recipe, 'meta.yaml')) as content:
def quote_var(match, quotes='"\''):
if (content[match.start() - 1] not in quotes and
content[match.end()] not in quotes):
return '"{}"'.format(match.group())
else:
return match.group()

# remove var defs to make yaml happy
content = JINJA_VAR_DEF.sub("", content.read())
# quote var usages if necessary to make yaml happy
content = JINJA_VAR.sub(quote_var, content)
# load as plain yaml
return yaml.load(content, Loader=yaml.BaseLoader)


def _superfluous_jinja_var(recipe, entry):
meta = load_plain_yaml(recipe)
m = meta
xpath = entry.split('/')
for p in xpath:
try:
m = m[p]
except KeyError:
return

match = JINJA_VAR.search(m)
if match:
return {
'unwanted_jinja_var': True,
'fix': 'replace jinja var {} in entry {} in '
'meta.yaml with a concrete value'.format(match.group(), entry)
}


def jinja_var_name(recipe, meta, df,):
return _superfluous_jinja_var(recipe, 'package/name')


def jinja_var_buildnum(recipe, meta, df,):
return _superfluous_jinja_var(recipe, 'build/number')


def jinja_var_version(recipe, meta, df):
return _superfluous_jinja_var(recipe, 'package/version')


def jinja_var_checksum(recipe, meta, df):
for checksum in ["sha256", "sha1", "md5"]:
ret = _superfluous_jinja_var(recipe, 'source/{}'.format(checksum))
if ret:
return ret


def missing_buildnum(recipe, meta, df):
meta = load_plain_yaml(recipe)
try:
meta['build']['number']
except KeyError:
return {
'missing_buildnum': True,
'fix': 'add build->number to meta.yaml'
}


@lint_multiple_metas
def in_other_channels(recipe, meta, df):
"""
Expand Down Expand Up @@ -405,5 +477,10 @@ def compilers_must_be_in_build(recipe, meta, df):
should_not_use_fn,
should_use_compilers,
compilers_must_be_in_build,
jinja_var_version,
jinja_var_buildnum,
jinja_var_name,
jinja_var_checksum,
missing_buildnum,
#bioconductor_37,
)
36 changes: 36 additions & 0 deletions bioconda_utils/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -1082,6 +1082,42 @@ def modified_recipes(git_range, recipe_folder, config_file):
return existing


def bump_build_numbers(recipes):
def bump(recipe):
if not recipe.endswith("meta.yaml"):
recipe = os.path.join(recipe, "meta.yaml")
state = 'general'
res = []
num = None
bumped = None
with open(recipe) as content:
for line in content:
if line.startswith('build'):
state = 'build'
elif state == 'build' and line.lstrip().startswith('number:'):
toks = line.split(':')
if toks == 1:
logger.warning("Failed to parse build number of recipe "
"%s: expected format 'number: value' "
"violated.", recipe)
return
try:
num = int(toks[1].strip())
except ValueError as e:
logger.warning("Failed to parse build number of "
"recipe %s: %s", recipe, e)
return
bumped = num + 1
line = '{}: {}\n'.format(toks[0], bumped)
res.append(line)
with open(recipe, "w") as out:
out.writelines(res)
logger.info("Bumped build number of recipe {} from {} to {}".format(recipe, num, bumped))

for recipe in recipes:
bump(recipe)


class Progress:
def __init__(self):
self.thread = Thread(target=self.progress)
Expand Down
54 changes: 54 additions & 0 deletions docs/source/linting.rst
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,60 @@ Rational: The compiler tools must not be in ``host:`` or ``run:`` sections.
How to resolve: Move ``{{ compiler() }}`` variables to the ``build:`` section.


`jinja_var_version`
~~~~~~~~~~~~~~~~~~~~
Reason for the failing: The recipe uses a jinja variable for defining the package version

Rationale: Using a jinja variable for package version is no longer necessary with conda-build 3, because the version can be referenced from all over the ``meta.yaml`` via the jinja expression ``{{ PKG_VERSION }}``.

How to resolve: Directly specify version, in the corresponding entry in the ``meta.yaml``.
Use the jinja expression ``{{ PKG_VERSION }}`` to refer to the version in other places of the ``meta.yaml``, e.g., in the ``url``.


`jinja_var_name`
~~~~~~~~~~~~~~~~

Reason for the failing: The recipe uses a jinja variable for package name.

Rationale: For package name, using a jinja variable has no benefit, because it is not supposed to change with updates.
Therefore, it is better for both human readability and parseability to stick to plain YAML syntax.

How to resolve: Directly specify name in the corresponding entry in the ``meta.yaml``.

`jinja_var_buildnum`
~~~~~~~~~~~~~~~~~~~~

Reason for the failing: The recipe uses a jinja variable for the build number.

Rationale: For build number, using a jinja variable has no benefit, because, although it changes during updates, it only appears at one place in the ``meta.yaml``.
Therefore, it is better for both human readability and parseability to stick to plain YAML syntax.

How to resolve: Directly specify build number in the corresponding entry in the ``meta.yaml``.

`jinja_var_checksum`
~~~~~~~~~~~~~~~~~~~~

Reason for the failing: The recipe uses a jinja variable for the source checksum.

Rationale: For checksum (sha1, sha256, or md5), using a jinja variable has no benefit, because, although it changes during updates, it only appears at one place in the ``meta.yaml``.
Therefore, it is better for both human readability and parseability to stick to plain YAML syntax.

How to resolve: Directly specify checksum in the corresponding entry in the ``meta.yaml``.


`missing_buildnum`
~~~~~~~~~~~~~~~
Reason for the failing: The recipe is missing a build number definition.

Rationale: Build number is crucial for building or updating the package.
Although conda would infer it automatically, it is for operational reasons beneficial
always have it explicitly defined in the recipe.



How to resolve: add ``.


..
`bioconductor_37`
~~~~~~~~~~~~~~~~~
Expand Down
155 changes: 155 additions & 0 deletions test/test_linting.py
Original file line number Diff line number Diff line change
Expand Up @@ -1019,6 +1019,161 @@ def test_should_not_use_fn():
)


def test_jinja_var_name():
run_lint(
func=lint_functions.jinja_var_name,
should_pass=[
'''
a:
meta.yaml: |
package:
name: a
version: 0.1
build:
number: 0
''',
],
should_fail=[
r'''
a:
meta.yaml: |
{% set name = "a" %}
package:
name: "{{ name|lower }}"
version: 0.1
build:
number: 0
''',
r'''
a:
meta.yaml: |
{% set name = "a" %}
package:
name: {{ name }}
version: 0.1
build:
number: 0
''',
]
)


def test_jinja_var_version():
run_lint(
func=lint_functions.jinja_var_version,
should_pass=[
'''
a:
meta.yaml: |
package:
name: a
version: 0.1
build:
number: 0
''',
],
should_fail=[
r'''
a:
meta.yaml: |
{% set version = "1.0" %}
package:
name: a
version: {{ version }}
build:
number: 0
''',
]
)


def test_jinja_var_buildnum():
run_lint(
func=lint_functions.jinja_var_buildnum,
should_pass=[
'''
a:
meta.yaml: |
package:
name: a
version: 0.1
build:
number: 0
''',
],
should_fail=[
r'''
a:
meta.yaml: |
{% set buildnum = 1 %}
package:
name: a
version: 0.1
build:
number: {{ buildnum }}
''',
]
)


def test_jinja_var_checksum():
run_lint(
func=lint_functions.jinja_var_checksum,
should_pass=[
'''
a:
meta.yaml: |
package:
name: a
version: 0.1
build:
number: 0
source:
sha256: abc
''',
],
should_fail=[
r'''
a:
meta.yaml: |
{% set sha256 = "abc" %}
package:
name: a
version: 0.1
build:
number: 0
source:
sha256: {{ sha256 }}
''',
]
)


def test_missing_buildnum():
run_lint(
func=lint_functions.missing_buildnum,
should_pass=[
'''
a:
meta.yaml: |
package:
name: a
version: 0.1
build:
number: 0
''',
],
should_fail=[
r'''
a:
meta.yaml: |
package:
name: a
version: 0.1
''',
]
)

#def test_bioconductor_37():
# run_lint(
# func=lint_functions.bioconductor_37,
Expand Down
Loading