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

wip - add WSL and SSH remotes #16109

Closed

Conversation

jcar87
Copy link
Contributor

@jcar87 jcar87 commented Apr 19, 2024

No description provided.

@davidsanfal davidsanfal force-pushed the feature/docker_wrapper branch 4 times, most recently from 931087b to 8fecd97 Compare April 22, 2024 16:36
Copy link
Member

@memsharded memsharded left a comment

Choose a reason for hiding this comment

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

Looking good!

@@ -0,0 +1,11 @@
FROM ubuntu:22.04
Copy link
Member

Choose a reason for hiding this comment

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

The 3 dockerfiles are almost identical, isn't it possible to layer them so testing with them is cheaper (faster images to build)?

Comment on lines +77 to +84
settings = "build_type", "compiler"
author = "John Doe"
license = "MIT"
url = "https://foo.bar.baz"
homepage = "https://foo.bar.site"
topics = "foo", "bar", "qux"
provides = "libjpeg", "libjpg"
deprecated = "other-pkg"
Copy link
Member

Choose a reason for hiding this comment

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

Most of the metadata, unless used in the test, can be removed to simplify the recipe.

Comment on lines +137 to +144
settings = "build_type", "compiler"
author = "John Doe"
license = "MIT"
url = "https://foo.bar.baz"
homepage = "https://foo.bar.site"
topics = "foo", "bar", "qux"
provides = "libjpeg", "libjpg"
deprecated = "other-pkg"
Copy link
Member

Choose a reason for hiding this comment

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

Same as above

@pytest.mark.parametrize("build_type,shared", [("Release", False), ("Debug", True)])
@pytest.mark.tool("ninja")
def test_create_docker_runner_with_ninja(build_type, shared):
conanfile = textwrap.dedent("""
Copy link
Member

Choose a reason for hiding this comment

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

Why not using a built-in conan new cmake_lib template?

Comment on lines +64 to +68
return {
'docker': DockerRunner,
'ssh': SSHRunner,
'wsl': WSLRunner,
}[profile_host.runner.get('type')](conan_api, 'create', profile_host, profile_build, args, raw_args).run()
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return {
'docker': DockerRunner,
'ssh': SSHRunner,
'wsl': WSLRunner,
}[profile_host.runner.get('type')](conan_api, 'create', profile_host, profile_build, args, raw_args).run()
try:
runner_class = { 'docker': DockerRunner, 'ssh': SSHRunner, 'wsl': WSLRunner}[profile_host.runner.get('type')]
except KeyError:
raise ConanException(f"Unrecognized runner 'type'={profile_host.runner.get('type')}")
runner = runner_class(conan_api, 'create', profile_host, profile_build, args, raw_args)
return runner.run()

A bit more UX-friendly ready, as users will miss or mistype the type.

@davidsanfal davidsanfal force-pushed the feature/docker_wrapper branch 2 times, most recently from a81379f to 07232cd Compare April 23, 2024 08:14
@czoido czoido deleted the branch conan-io:feature/docker_wrapper May 6, 2024 14:26
@czoido czoido closed this May 6, 2024
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.

None yet

4 participants