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

project loader, schema: add advanced grammar support for build-environment #3350

Merged
merged 4 commits into from
Jan 14, 2021

Conversation

cjp256
Copy link
Contributor

@cjp256 cjp256 commented Nov 4, 2020

No description provided.

@cjp256
Copy link
Contributor Author

cjp256 commented Nov 6, 2020

Grammar changes are complete, I'm going to split that into it's own PR, with build-environment changes coming separately.

@cjp256 cjp256 marked this pull request as ready for review November 9, 2020 22:37
@cjp256 cjp256 changed the title [WIP] project loader: advanced grammar for build-environment project loader, schema: add advanced grammar support for build-environment Nov 9, 2020
@codecov-io
Copy link

codecov-io commented Dec 15, 2020

Codecov Report

Merging #3350 (76c2f7b) into master (a05e645) will decrease coverage by 0.01%.
The diff coverage is 90.90%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3350      +/-   ##
==========================================
- Coverage   90.75%   90.73%   -0.02%     
==========================================
  Files         257      256       -1     
  Lines       18230    18206      -24     
==========================================
- Hits        16544    16520      -24     
  Misses       1686     1686              
Impacted Files Coverage Δ
snapcraft/internal/pluginhandler/__init__.py 90.56% <50.00%> (+0.04%) ⬆️
snapcraft/internal/project_loader/_parts_config.py 97.63% <100.00%> (+0.01%) ⬆️
...ader/grammar_processing/_part_grammar_processor.py 100.00% <100.00%> (ø)
snapcraft/project/_project_options.py 90.71% <0.00%> (-0.84%) ⬇️
snapcraft/_store.py 86.55% <0.00%> (-0.22%) ⬇️
snapcraft/cli/assertions.py 97.93% <0.00%> (-0.03%) ⬇️
...pcraft/internal/pluginhandler/_part_environment.py 49.05% <0.00%> (ø)
.../internal/project_loader/_extensions/gnome_3_34.py 100.00% <0.00%> (ø)
.../internal/project_loader/_extensions/gnome_3_38.py

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ceee8c0...587c4f1. Read the comment docs.

…nment

Drop the _list_of_dicts_to_env() conversion and account for
the data structure as-is when using it.

Signed-off-by: Chris Patterson <chris.patterson@canonical.com>
@sergiusens
Copy link
Collaborator

Units are failing

@@ -646,7 +641,7 @@ def _generate_part_env(self, step: steps.Step) -> str:
for k, v in plugin_environment.items():
print(f'export {k}="{v}"', file=run_environment)
print("## User Environment", file=run_environment)
for env in user_build_environment:
for env in self.build_environment:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
for env in self.build_environment:
# Part's (user) say.
for env in self.build_environment:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@@ -646,7 +641,7 @@ def _generate_part_env(self, step: steps.Step) -> str:
for k, v in plugin_environment.items():
print(f'export {k}="{v}"', file=run_environment)
print("## User Environment", file=run_environment)
for env in user_build_environment:
for env in self.build_environment:
Copy link
Collaborator

Choose a reason for hiding this comment

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

or (I prefer this even if it is more code) combined with the change some lines of code prior

Suggested change
for env in self.build_environment:
for env in user_build_environment:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@@ -630,9 +628,6 @@ def _generate_part_env(self, step: steps.Step) -> str:
else:
plugin_environment = dict()

# Part's (user) say.
user_build_environment = self._part_properties["build-environment"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

code suggestions not working for removed lines, I would keep user_build_environment but = self.build_environment

This makes the code obvious to casual readers. The "user" part of this variable name was your idea ;-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, will do

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Chris Patterson added 2 commits January 14, 2021 12:35
Signed-off-by: Chris Patterson <chris.patterson@canonical.com>
@cjp256
Copy link
Contributor Author

cjp256 commented Jan 14, 2021

Only test failing is plainbox.

@sergiusens sergiusens merged commit ef2fa17 into canonical:master Jan 14, 2021
abitrolly pushed a commit to abitrolly/snapcraft that referenced this pull request Mar 31, 2021
…nment (canonical#3350)

Drop the _list_of_dicts_to_env() conversion and account for
the data structure as-is when using it.

Signed-off-by: Chris Patterson <chris.patterson@canonical.com>
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.

3 participants