Skip to content

Conversation

@jonpryor
Copy link
Contributor

@jonpryor jonpryor commented Mar 25, 2020

When Directory.Build.props is imported by MSBuild, the
$(Configuration) property will only be defined if it was excplitily
provided on the command line. As such, when building the repo when
using make(1), it builds:

    $ make prepare
    $ make all
    # works!

After make all, if we attempt to manually build a single .csproj,
it may fail:

    $ msbuild tests/Java.Interop-Tests/Java.Interop-Tests.csproj
    ...
    BuildInteropTestJar:
      "" -source 1.6 -target 1.6 -bootclasspath "" -d "obj/Debug/it-classes" -classpath …
    …: error MSB3073: The command """ -source 1.6 -target 1.6 …" exited with code 127.

Building a single .csproj works if the Configuration property is
explicitly provided:

    $ msbuild tests/Java.Interop-Tests/Java.Interop-Tests.csproj /p:Configuration=Debug
    # no error

Update Directory.Build.props so that if no $(Configuration) is
provided we default to the Debug configuration. This allows all the
relevant $(Configuration)-dependent properties to have correct and
consistent values, which allows projects to build as intended:

    $ msbuild tests/Java.Interop-Tests/Java.Interop-Tests.csproj
    # no errors

@jonathanpeppers
Copy link
Member

I was hitting this locally yesterday, and I just changed Directory.Build.props:

-    <_Configuration Condition=" '$(Configuration)' == 'Gendarme' ">Debug</_Configuration>
+    <_Configuration Condition=" '$(Configuration)' == 'Gendarme' Or '$(Configuration)' == '' ">Debug</_Configuration>

I don't know if this has another issue though ^^

@jpobst
Copy link
Contributor

jpobst commented Mar 25, 2020

Anything with Gendarme can be killed, as we no longer use it. I think Peppers may be on to something, and we could just add the standard setter at the top of Directory.Build.props:

<Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
<Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>

@jonpryor jonpryor force-pushed the jonp-output-paths branch from 437e03d to a2fc2fd Compare March 25, 2020 15:58
When `Directory.Build.props` is imported by MSBuild, the
`$(Configuration)` property will only be defined if it was excplitily
provided on the command line.  As such, when building the repo when
using **make**(1), it builds:

	$ make prepare
	$ make all
	# works!

After `make all`, if we attempt to manually build a single `.csproj`,
it may fail:

	$ msbuild tests/Java.Interop-Tests/Java.Interop-Tests.csproj
	...
	BuildInteropTestJar:
	  "" -source 1.6 -target 1.6 -bootclasspath "" -d "obj/Debug/it-classes" -classpath …
	…: error MSB3073: The command """ -source 1.6 -target 1.6 …" exited with code 127.

Building a single `.csproj` works if the `Configuration` property is
explicitly provided:

	$ msbuild tests/Java.Interop-Tests/Java.Interop-Tests.csproj /p:Configuration=Debug
	# no error

Update `Directory.Build.props` so that if no `$(Configuration)` is
provided we default to the Debug configuration.  This allows all the
relevant `$(Configuration)`-dependent properties to have correct and
consistent values, which allows projects to build as intended:

	$ msbuild tests/Java.Interop-Tests/Java.Interop-Tests.csproj
	# no errors
@jonpryor jonpryor force-pushed the jonp-output-paths branch from a2fc2fd to 9d47269 Compare March 25, 2020 17:27
@jonpryor jonpryor changed the title [build] Add build-tools/scripts/configuration-paths.props [build] Provide a default $(Configuration) value Mar 25, 2020
Copy link
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

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

👍 LTGM if it builds

@jonpryor jonpryor merged commit 3091274 into dotnet:master Mar 25, 2020
@jpobst jpobst added this to the d16-7 milestone Apr 10, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Apr 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants