MSBuildHelper: new property ToolPath

[0x53A]

I need CI, CI, CI is what I need 💲 💵 💲

[baronfel]

Love it. Slight nitpick: maybe expand your comment on toolpath to tell the user that the value can be a path and if so the executable will be inferred according to the rules you've used? One thing I regret from the old msbuild resolution is that the rules were documented in internal code comments but not surfaced to the users.

[0x53A]

I actually don't like Toolpath accepting a directory. The user should always provide the full path to the exe.

I only did it this way because the existing ToolPaths in the code base are inconsistent between dir and file, and because the environment variable MSBuild also accepts both.

[0x53A]

/poke @matthid

[matthid]

?

Is there an open question or something?
Will take a look when I have some time ;)

[matthid]

Well, it looks good, but the same changes have to go into the FAKE 5 module (or only there actually): here

The MsBuildHelper is obsolete (yes we forgot to add the obsolete attributes - sorry about that, please feel free to add them ;))

[matthid]

And sorry for the very long delay ...

[0x53A]

Well, it looks good, but the same changes have to go into the FAKE 5 module (or only there actually): here

Q:

Will there be another release for Fake 4.x? If yes, I would like this change to be included.
If no, then whatever.

The MsBuildHelper is obsolete (yes we forgot to add the obsolete attributes - sorry about that, please feel free to add them ;))

Will do

And sorry for the very long delay ...

No problem, I just took the version from the CI server. IF I had really needed it, I would have pinged you again ;D

Thanks!

[matthid]

Will there be another release for Fake 4.x? If yes, I would like this change to be included.
If no, then whatever.

@forki likes to do them for hotfixes if needed. I personally have technical problems releasing FAKE 4 (something always fails and I basically have given up). But I also don't really see the reason, if you ignore the obsolete attributes then the v5 version of the nuget package should be exactly the same, so as long as you don't upgrade to new API (ie ignore the obsolete warning) you are on the stable branch.
Maybe you can hook in --nowarn:44 or something to even get rid of the warnings.
If we broke something with the v5 package in the OLD API we need to consider to revert that change.

I would have pinged you again ;D

Yes sometimes that is indeed needed, thanks ;)

[0x53A]

but the same changes have to go into the FAKE 5 module

done, I changed both.

The MsBuildHelper is obsolete

added attribute.

---

I also changed my opinion regarding allowing directories:

In my opinion the user should always specify the full path, guessing an executable name doesn't make much sense.

So I removed that logic.

What do you think? If you really want, I can add it back.

[matthid]

Changes look good to me! I'll add it into the next release/round of changes.

What do you think? If you really want, I can add it back.

I don't have a strong opinion either way, only that specifying the full executable path should probably work (which it does at the moment).

Thanks for taking care of this.