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

Standard build variables are not replaced in before/after build scripts. #677

Open
TheRealPiotrP opened this issue Jan 18, 2017 · 20 comments

Comments

Projects
None yet
@TheRealPiotrP
Copy link
Contributor

commented Jan 18, 2017

Moving from dotnet/cli#5095 on behalf of @mdekrey


Sorry if this isn't the right spot; I wasn't sure if the MSBuild repo was a better location for this report or not. Simplest steps are below, but I originally accessed this via Visual Studio, which gave menus for the "macro" variables that did not get replaced. Note that this occurs for both project variables, not just solution variables, as the example uses the dotnet cli.

Steps to reproduce

  1. Create a new project (dotnet new)
  2. Modify your .csproj to have PreBuildEvent or PostBuildEvent that uses build variables as documented in previous C# projects. For example:
    <PropertyGroup Label="Configuration">
      <PreBuildEvent>echo $(OutDir)</PreBuildEvent>
      <PostBuildEvent>echo $(OutDir)</PostBuildEvent>
    </PropertyGroup>
  1. Restore packages. (dotnet restore)
  2. Build the project. (dotnet build)

Expected behavior

Standard build variables should be replaced in before/after build scripts.

Microsoft (R) Build Engine version 15.1.458.808
Copyright (C) Microsoft Corporation. All rights reserved.

  bin\Debug\netcoreapp1.0\
  test -> C:\Users\Me\Source\test\bin\Debug\netcoreapp1.0\test.dll
  bin\Debug\netcoreapp1.0\

Actual behavior

Standard build variables are not replaced in before/after build scripts.

Microsoft (R) Build Engine version 15.1.458.808
Copyright (C) Microsoft Corporation. All rights reserved.

  ECHO is on.
  test -> C:\Users\Me\Source\test\bin\Debug\netcoreapp1.0\test.dll
  ECHO is on.

Environment data

dotnet --info output:

.NET Command Line Tools (1.0.0-preview4-004233)

Product Information:
 Version:            1.0.0-preview4-004233
 Commit SHA-1 hash:  8cec61c6f7

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.14393
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\1.0.0-preview4-004233

@srivatsn srivatsn added this to the 1.0 RTM milestone Jan 18, 2017

@srivatsn srivatsn added the Bug label Jan 18, 2017

@srivatsn

This comment has been minimized.

Copy link
Collaborator

commented Jan 18, 2017

Not sure if this is a SDK or a project system issue. @basoundr can you take a look?

@dsplaisted

This comment has been minimized.

Copy link
Member

commented Jan 18, 2017

This is because the OutDir and similar properties are set in the SDK targets, so they are not set when the PreBuildEvent and PostBuildEvent properties are evaluated in the project file.

I think for this to work in the previous project system, you would have to set the build event properties after the CSharp.targets import at the bottom of your file. This isn't easy to do now that that import is implicit.

In light of this, we may want to discourage or disable use of PreBuildEvent and PostBuildEvent in SDK projects. The alternative would be to define Targets in the project file, as properties inside those Targets would be evaluated after the SDK .targets files have been evaluated and set the properties.

@rainersigwald

This comment has been minimized.

Copy link
Contributor

commented Jan 18, 2017

I'd love to disable use of the events in general for many reasons like this. Disabling them for Sdk projects gets a 👍 from me.

@srivatsn srivatsn modified the milestones: 1.1, 1.0 RTM Jan 19, 2017

@Ziflin

This comment has been minimized.

Copy link

commented Feb 11, 2017

Is there any solution for this? It is affecting us as well. We were also trying to change the Output path in the project settings, but clicking the Browse button crashes VS.

@mgwalm

This comment has been minimized.

Copy link

commented Feb 15, 2017

You cant just decide to get rid of this. I think thousands of users all over the world use it.

@Ziflin

This comment has been minimized.

Copy link

commented Feb 15, 2017

I'm still trying to find out when this will be fixed as we definitely need the pre/post build events to work and to properly pass in the macros/environment variables?

@dsplaisted

This comment has been minimized.

Copy link
Member

commented Feb 15, 2017

@mgwalm @Ziflin I don't expect us to make PreBuildEvent and PostBuildEvent get the final values of properties as OutDir. If anything we would disable them entirely in .NET SDK projects to avoid people hitting this issue.

Instead, what you can do is use targets to do this. Here's an example:

  <Target Name="PreBuild" BeforeTargets="PreBuildEvent">
    <Exec Command="echo Before Build: $(OutDir)" />
  </Target>

  <Target Name="PostBuild" AfterTargets="PostBuildEvent">
    <Exec Command="echo After Build: $(OutDir)" />
  </Target>
@Ziflin

This comment has been minimized.

Copy link

commented Feb 15, 2017

@dsplaisted Thanks! That's all I was looking for. Is there a webpage somewhere that describes the new csproj format?

@dsplaisted

This comment has been minimized.

Copy link
Member

commented Feb 15, 2017

@Ziflin

This comment has been minimized.

Copy link

commented Feb 15, 2017

@dsplaisted Great! Thank you very much!

@StingyJack

This comment has been minimized.

Copy link

commented Apr 25, 2017

This still doesn't work for things like $(ConfigurationName) either, and that is known before building.

@livarcocc livarcocc modified the milestones: 2.0.0, 2.1.0, 2.2.0 May 23, 2017

@darkurse

This comment has been minimized.

Copy link

commented Jun 21, 2017

@StingyJack , if it helps this actually worked for me when combined with the solution in issue #1569 ( anwer from davkean ).

The only drawback for me is that these prebuild and postbuild targets are not visible/editable from vs2017 prroject settings UI. And so if I don't tell my colleagues about this fact they won't see my changes and will start believing in black magic :)

@rainersigwald

This comment has been minimized.

Copy link
Contributor

commented Jun 21, 2017

With dotnet/project-system#2367, the Visual Studio UI will now emit targets for build customization, so adding new steps to new projects will allow use of any properties and items defined at the relevant point in the build.

The new implementation does NOT update existing definitions of the *Event properties, so you might need to remove them and re-add them through the UI to get the updated behavior. If you already defined your own target, 1) 👍and 2) you shouldn't need to take any action, unless you really want to use the UI to manage the process.

@DaveInCaz

This comment has been minimized.

Copy link

commented Aug 22, 2018

Is this the same issue as dotnet/project-system#1569 ?

@rainersigwald

This comment has been minimized.

Copy link
Contributor

commented Aug 22, 2018

@DaveInCaz Not exactly. That bug was fixed when Visual Studio was taught to emit pre- and post-build events using a better mechanism that isn't subject to the constraints discussed here. However, it's still possible to manually use the property approach, which is subject to the ordering-of-property-definitions constraints that cause problems here.

@StingyJack

This comment has been minimized.

Copy link

commented Aug 23, 2018

unless you really want to use the UI to manage the process.

Well, yes. I dont really want to do all the typing required to unload, manually modify a proj file, and then reload it when I can do it in a few shortcut keys. I sure as heck dont want to type in full commands for anything. We didn't pay for Visual Studio so we could have notepad + msbuild.

I'd love to disable use of the events in general for many reasons like this

That doesn't read very well.

I don't expect us to make PreBuildEvent and PostBuildEvent get the final values of properties as OutDir. If anything we would disable them entirely in .NET SDK projects to avoid people hitting this issue.

The issue is that the things dont populate as expected. They are useful things to many people, and I dont see a compelling reason here to make all those users (customers) have to change what was working code before that is now not.

@Rabadash8820

This comment has been minimized.

Copy link

commented Sep 12, 2018

Adding to @StingyJack's points...

I have zero problem with build events being phased out and build targets being the new best practice, especially if the implementation is as simple as dsplaisted's answer above. All of us long-time Microsoft users could stand to get out of our comfort zone and write some code rather than having a magical UI take care of everything for us, and I love that the "new" Microsoft seems to be embracing more transparent tools that can be tweaked in any text editor, not just with mindless clicking.

All that said, I have two problems with the current build-event situation:

  1. Build events are still present and editable in the project Properties page. If we are supposed to be moving towards build targets, this UI should be removed. People are going to keep using a tool as long as it is available, even if they're not supposed to (myself included, until I saw this Issue).
  2. If this UI is not going to be removed, then it needs to work! What we have right now is a massively breaking change for all moderately complex build pipelines, with no documentation that I know of other than 1-2 year old GitHub issues. That this silent bug still exists in VS 15.8 is embarassing; all you need to add is a notice to the Build Events Properties tab saying "Warning: build variables are not replaced in .*proj files targeting .NET Standard" or a green squiggly in the .*proj editor with the same message.
@dasMulli

This comment has been minimized.

Copy link
Contributor

commented Sep 12, 2018

@Rabadash8820 Projects using the new project system already get the some new behaviour: The UI will actually create targets. This was added in 15.3 - dotnet/project-system#2367. But AFAIK it doesn't switch from the build events to targets, but it will create and edit targets when you first use it.

@DaveInCaz

This comment has been minimized.

Copy link

commented Sep 12, 2018

@dasMulli how does a version # like 15.3 which you mentioned correspond to what is available to a given version of Visual Studio? Thanks

@StingyJack

This comment has been minimized.

Copy link

commented Sep 12, 2018

@DaveInCaz - 15.x is the version number of VS 2017
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.