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 Dotnet plugin and a sample snap for integration test #1574
Add Dotnet plugin and a sample snap for integration test #1574
Conversation
51aa27c
to
b269b07
Compare
@ElOpio , What's the next step for this PR ? I see CI failing with test_list_plugins_large_terminal test. |
b269b07
to
83d2bed
Compare
snapcraft/plugins/dotnet.py
Outdated
|
||
# These are required for the runtime | ||
if 'no-runtime' in options.build_attributes: | ||
self._runtime = _DummyTar() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this self._runtime is not being used anywhere.
snapcraft/plugins/dotnet.py
Outdated
try: | ||
# TODO support more SDK releases | ||
sdk_version = sdk_arch['2.0.0'] | ||
except KeyError as missing_version: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an exception that will never be reached, because the '2.0.0' is hardcoded.
snapcraft/plugins/dotnet.py
Outdated
def schema(cls): | ||
schema = super().schema() | ||
|
||
schema['properties']['dotnet-runtime'] = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This property is never used in the plugin.
snapcraft/plugins/dotnet.py
Outdated
|
||
if 'no-runtime' in self.options.build_attributes: | ||
os.makedirs(os.path.join(self.installdir, 'dotnet-runtime'), | ||
exist_ok=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no-runtime means only that a dir has to be created? That sounds weird.
snapcraft/plugins/dotnet.py
Outdated
exist_ok=True) | ||
else: | ||
# Workaround to set the right permission for the executable. | ||
appname = os.path.join(self.installdir, self._get_appname()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if _get_appname doesn't return anything, this call will fail.
Hello @rakeshsinghranchi! I was still traveling yesterday. I've just pushed the tests I wrote, and left a few comments in the PR for you and @sergiusens to look at. It seems to me that it will be easier now to remove everything related to 'no-runtime' in this PR, and add it in a follow up, with an integration test. |
snapcraft/plugins/dotnet.py
Outdated
break | ||
|
||
def env(self, root): | ||
env = super().env(root) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems to me that this env is never used by the plugin.
83d2bed
to
33d9a62
Compare
To make this easier to land asap, I've pushed a new commit that removes the unused code, mostly references to runtime. In follow up PRs we can add the option to select a dotnet version, and no-runtime. This is the commit with the removed code, so we don't lose it: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for getting this green @ElOpio
@rakeshsinghranchi thanks for the inital work
As a followup we should either pickup the sdk from the releases json file but maybe default to the dotnet-sdk snap as a build-snap
Thanks so much @ElOpio and @sergiusens for helping throughout this PR. @sergiusens , this PR takes care of building Self-Contained application. We would need "no-runtime" ( or equivalent enum ) build attribute or plugin specific keyword to support FDD ( Framework dependent deployment) scenario too. Will work on picking up SDK from releases json file and default to dotnet-sdk snap as build-snap. |
./runtests.sh static
?./runtests.sh unit
?