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

- conan profile new --detect for MSYS #2798

Merged
merged 1 commit into from
Apr 27, 2018
Merged

Conversation

SSE4
Copy link
Contributor

@SSE4 SSE4 commented Apr 24, 2018

Signed-off-by: SSE4 tomskside@gmail.com

  • Refer to the issue that supports this Pull Request.
  • If the issue has missing info, explain the purpose/use case/pain/need that covers this Pull Request.
  • I've read the Contributing guide.
  • I've followed the PEP8 style guides for Python code.
  • I've opened another PR in the Conan docs repo to the develop branch, documenting this one. Also adding a description of the changes in the changelog.rst file. https://github.com/conan-io/docs

Closes #2799 Closes #2638

@ghost ghost added the contributor pr label Apr 24, 2018
@lasote lasote added this to the 1.3 milestone Apr 25, 2018
return result


def detect_subsystem():
Copy link
Contributor

@lasote lasote Apr 25, 2018

Choose a reason for hiding this comment

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

I'm wondering if we should improve

def detect_windows_subsystem():
.
to also detect when you are actually running conan inside the subsystem, and not only having a bash available.
From the recipes point of view if I ask self.os_info.detect_windows_subsystem() it should return the subsystem I'm running. It will increase the compatibility of the recipes with Conan running inside the subsystem.
But I'm not saying that we should use the detect_windows_subsystem without changes in this detection because here is important to distinguish that we are running INSIDE the subsystem.

But anyway, look at my following comment, I'm not sure if we should adjust the subsystem subsetting even detecting we are running inside a subsystem.

@@ -248,6 +264,9 @@ def _detect_os_arch(result, output):
the_os = detected_os()
result.append(("os", the_os))
result.append(("os_build", the_os))
the_subsystem = detect_subsystem()
if the_subsystem:
result.append(("os.subsystem", the_subsystem))
Copy link
Contributor

Choose a reason for hiding this comment

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

This subsetting was invented to distinguish the "native" built apps for the subsystem (using internal dll) or native windows apps.
That you are running Conan in a subsystem it doesn't mean that you are building subsystem-native apps, for example, you can run MSYS2 in mingw mode to build windows native apps.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so, remove it?

Copy link
Contributor

Choose a reason for hiding this comment

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

I would say, yes, remove it. The subsystem subsetting shouldn't be autodetected.

Copy link
Contributor

Choose a reason for hiding this comment

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

If you can remove it today (any time) we can add it to 1.3

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, 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.

@lasote done

if result.startswith("MINGW32_NT") or result.startswith("MINGW64_NT"):
return "Windows"
if result.startswith("MSYS_NT"):
return "Windows"
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm ok with this.

@lasote lasote modified the milestone: 1.3 Apr 25, 2018
Signed-off-by: SSE4 <tomskside@gmail.com>
@lasote lasote merged commit f439628 into conan-io:develop Apr 27, 2018
@ghost ghost removed the contributor pr label Apr 27, 2018
grisumbras pushed a commit to grisumbras/conan that referenced this pull request Dec 27, 2018
Signed-off-by: SSE4 <tomskside@gmail.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.

2 participants