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

Add command line options to manipulate imports #3691

Open
yatli opened this issue Sep 2, 2018 · 14 comments
Open

Add command line options to manipulate imports #3691

yatli opened this issue Sep 2, 2018 · 14 comments
Labels

Comments

@yatli
Copy link

yatli commented Sep 2, 2018

msbuild implicitly import files during various stages. there is no (obvious) way to inspect or modify what is imported, at the moment.

for example (dotnet/sdk#887), sometimes we find overriding a property with /p flag is too late, but the only chance of early import relies on heuristics in finding Directory.Build.props -- a wildcard that might be "too greedy" (#762).

can we get command line options to manipulate such behaviors? e.g. if specified, overrides the above heuristic and use a given file instead, or list the files imported, etc.

Also see #3166 and https://github.com/dotnet/roslyn/blob/master/src/Workspaces/CoreTestUtilities/Resources/Directory.Build.props

@dasMulli
Copy link
Contributor

dasMulli commented Sep 3, 2018

In theory, -p:DirectoryBuildPropsPath=/path/to/some.props should already work.

Would be interesting to change _DirectoryBuildPropsFile to a public property to allow changing the file name.
Currently -p:_DirectoryBuildPropsFile=foo.props should already work and change the file name from Directory.Build.props to foo.props while maintaining the hierarchy search logic.
Is that what you are looking for?

@dasMulli
Copy link
Contributor

dasMulli commented Sep 3, 2018

There's also a ImportDirectoryBuildProps one could set to false

@yatli
Copy link
Author

yatli commented Sep 3, 2018

hi @dasMulli!

Yeah, that property would solve my problem.
I remember seeing this somewhere, but was too afraid that a /p flag will come in too late. Will give it a try.
Also, we could figure out what's imported via /pp and look through the import comments. It's just not that obvious. :)

btw, will it still look up the hierarchy until root if I override the DirectoryBuildPropsPath with a custom name? from your example, intuitively, it won't do this because an absolute path is given.

@dasMulli
Copy link
Contributor

dasMulli commented Sep 3, 2018

No, DirectoryBuildPropsPath is the full path, _DirectoryBuildPropsFile would do the lookup but it's not really a public property (hence the underscore).
It would be interesting if MSBuild maintainers would be accepting a change to make this property public (remove the underscore).
Personally, I'd rather not and require users to always use Directory.Build.props for consistency. In there, build authors can add additional logic to search for other files or import other known files.

/p(-p) sets global properties which are present from the beginning and can't be overwritten by the static portion of project files (with an exception involving <Project TreatAsLocalProperty="…">) so you don't need to worry about it applying too late.

@yatli
Copy link
Author

yatli commented Sep 4, 2018

overriding DirectoryBuildPropsPath works for me, thanks @dasMulli !

I too would rather not let users create arbitrary recursive lookups. Directory.Build.props at least looks like a build system-related file, while arbitrary .props file may be everywhere in the filesystem.

I'm still not very sure about /p flag (also discussed here: #1603)

If I specify /p:BaseIntermediateOutputPath= at command line, I get inconsistent result -- the intermediate path indeed moved to the desired location, but project.json etc. remained in the obj folder, hence my concern about "too late". :)

@dasMulli
Copy link
Contributor

dasMulli commented Sep 4, 2018

The BaseIntermediateOutputPath trouble isn't/wasn't related to the order of definitions but rather NuGet and MSBuild using different properties for essentially the same concerns.
If you still have troubles with it, I'd suggest logging a fresh issue somewhere to discuss implications. The most difficult part about it is making sure that restore runs and builds have the same global properties set via CLI parameters. I believe Directory.Build.props may be the best option here to ensure a consistent experience.

Directory.Build.props at least looks like a build system-related file

It is also more in-line with Directory.Build.rsp which is not implemented in MSBuild files (common props/targets) but in MSBuild itself, thus the name can't really be changed in a "good/consistent/compatible" way.

@dasMulli
Copy link
Contributor

dasMulli commented Sep 4, 2018

ad BaseIntermediateOutputPath: also see #3695.
TL;DR don't set it for solutions.
But I think it should work for stand-alone projects. Or not using a rooted property / common location.

@yatli
Copy link
Author

yatli commented Sep 6, 2018

The issue #3695 describes a problem very similar to mine. Even if I both override BaseIntermediateOutputPath and then specify /p during a dotnet restore proj_path, it simply ignores the settings and writes project.assets.json and the friends to proj/obj/.

@yatli
Copy link
Author

yatli commented Sep 12, 2018

I think this issue can be closed now -- recommend documenting DirectoryBuildPropsPath somewhere though. Thanks @dasMulli for clarifying everything!

@yatli
Copy link
Author

yatli commented Sep 12, 2018

btw I'm rolling a cmake-dotnet integration module, FYI:

Z3Prover/z3#1423

https://github.com/Microsoft/GraphEngine/blob/master/cmake/FindDotnet.cmake

@yatli
Copy link
Author

yatli commented Nov 7, 2018

@dasMulli some new findings here, even if I override DirectoryBuildPropsPath, during package restore, Directory.Build.props is choosen and my custom props file is ignored.

This brings complexity to parameterized package reference, e.g. <PackageReference Include="..." Version="$(SOME_DEP_VERSION)" /> -- as our package version is refreshed on build, we have to update the root Directory.Build.props also on build -- and this breaks the out-of-tree building principle.

(edited)

@dasMulli
Copy link
Contributor

dasMulli commented Nov 7, 2018

@yatli that sounds more like a caching issue.. do you use -restore to restore and build during the build or -t:Restore;Build? (which doesn't work if you change xml content in between)
Do you have a sample project that reproduces this behaviour?

@yatli
Copy link
Author

yatli commented Nov 8, 2018

@dasMulli I use dotnet restore /p:.....

looks like DirectoryBuildPropsPath expects a full path, and it correctly picks up the custom props when I give it a proper path.

@dasMulli
Copy link
Contributor

dasMulli commented Nov 8, 2018

yeah if it isn't a full path, it will be relative to the project directory. And if you do it on a solution, it will be relative to each project individually..

@AR-May AR-May added the triaged label Feb 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants