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

msbuild temp props in build folder #4113

Merged
merged 6 commits into from Dec 12, 2018

Conversation

lasote
Copy link
Contributor

@lasote lasote commented Dec 12, 2018

Changelog: Fix: The property file that the MSBuild() is now generated in the build_folder instead of a temporary folder to allow more reproducible builds.
Docs: conan-io/docs#980

Closes #4085

PENDING DOCS

@ghost ghost assigned lasote Dec 12, 2018
@ghost ghost added the stage: review label Dec 12, 2018
@lasote lasote added this to the 1.11 milestone Dec 12, 2018
@lasote lasote assigned jgsogo and memsharded and unassigned lasote Dec 12, 2018
@lasote lasote requested a review from danimtb December 12, 2018 11:30
@lasote lasote changed the title msbuild temp props build in build folder msbuild temp props in build folder Dec 12, 2018
danimtb
danimtb previously approved these changes Dec 12, 2018
@danimtb
Copy link
Member

danimtb commented Dec 12, 2018

maybe add a check in the existing tests to check file is generated in the build folder now using a custom property_file_name?

@lasote lasote dismissed danimtb’s stale review December 12, 2018 11:39

Missing test is important

@ghost ghost assigned lasote Dec 12, 2018
Copy link
Contributor

@jgsogo jgsogo left a comment

Choose a reason for hiding this comment

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

Looks good!

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.

Please remove tmp_file dead code and merge.

self.build_env.parallel = parallel

with tools.environment_append(self.build_env.vars):
# Path for custom properties file
props_file_contents = self._get_props_file_contents()
with tmp_file(props_file_contents) as props_file_path:
Copy link
Member

Choose a reason for hiding this comment

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

The tmp_file context manager is now dead code, please remove it.

@memsharded memsharded merged commit 663a887 into conan-io:develop Dec 12, 2018
@ghost ghost removed the stage: review label Dec 12, 2018
grisumbras pushed a commit to grisumbras/conan that referenced this pull request Dec 27, 2018
…build

msbuild temp props in build folder
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