conan-io / conan-vs-extension

Conan Extension for Visual Studio
https://marketplace.visualstudio.com/items?itemName=conan-io.conan-vs-extension
MIT License
59 stars 34 forks source link

The .conan directory is generated in the project's output directory #127

Closed LBHawk closed 5 years ago

LBHawk commented 5 years ago

The .conan directory appears to be written to $(OutDir), which is not desirable. The .conan directory should instead be generated in the project root.

image

memsharded commented 5 years ago

The problem here is that there is a different conanbuildinfo.props for each different configuration. This cannot go to the project root, because they will collide, this file needs to be put in the OutDir of every configuration. Plus the added effect that this is a temporary file, and putting it into the OutDir will most likely automatically exclude it from version control.

I don't know the details of the plugin, I might be missing something, could you please clarify why should that be moved to the project root? Thanks!

icedream2linxi commented 5 years ago

OutDir is the project generation file storage directory. Usually multiple projects are generated into the same directory. I recommend changing the .conan directory to configurable. It is up to the user to decide whether it is $(OutDir) or $(ProjectDir).

OutDir 是项目生成文件存放目录。通常多个项目会生成到同一目录下。 我建议 .conan 目录改成可配置。由用户决定是 $(OutDir) 还是 $(ProjectDir)。

jodylittle commented 5 years ago

I'm also experiencing this issue. My solution contains two projects, each with their own conanfile and unique package requirements.

Running Install (Current Project) generates the proper conanbuildinfo.props in the output folder in the default location: $(SolutionDir)$(Platform)\$(Configuration).

Now running Install (Current Project) on the second project will overwrite that conanbuildinfo.props.

The Intermediate Directory can have a similar problem because it also defaults to a common location in the solution. Micrsoft issues a warning (MSB8028) in that case, which I usually fix by changing the Intermediate Directory to $(ProjectDir)$(Platform)\$(Configuration).

As @icedream2linxi stated, if the .conan directory location was configurable, it could be set to $(OutDir), $(IntDir) or some other location.

SSE4 commented 5 years ago

hi @LBHawk , @icedream2linxi and @jodylittle thanks a lot for your feedback I have implemented required changes in branch, and it would be nice if you could download .vsix file and try it out: https://ci.appveyor.com/project/ConanOrgCI/conan-vs-extension/builds/25773658/artifacts now conan installation directory is configurable, by going to the Tools -> Options -> Conan -> Conan installation directory you may specify custom location. It could be an absolute path, or relative (which is relative to the current project). also, you may use Visual Studio macro definitions, like $(OutDir), $(IndDir), $(ProjectDir), $(SolutionDir) and so on. Full list is here: https://docs.microsoft.com/en-gb/cpp/build/reference/common-macros-for-build-commands-and-properties?view=vs-2019 By default, conan uses $(OutDir).conan. You may use, for instance, $(SolutionDir).my_conan_deps, if you want to share the same conan dependencies between all solution projects. Please try to download and install the extension from the link above, and lemme know if it works for you.

memsharded commented 5 years ago

Great!

Just beware that that is the location where the "conanbuildinfo.props" file for every configuration for every conanfile is written. If you use the same location for different configurations or conanfiles, it will probably be overwritten, and something will fail

jodylittle commented 5 years ago

Thanks @SSE4, I installed the new extension. It works well and does solve this issue for me.

I noticed if a macro fails to evaluate, it is replaced with an empty string. The installation will continue into an unexpected location. For example if I use $(BadValue)\, it would install to root.

Perhaps better to throw an error. Or maybe just output the install location to the console to help with diagnosing.

@memsharded, agreed there is a risk, but this change at least makes it solvable. It may be possible to determine if multiple configurations are using the same installation directory and issue a warning.

memsharded commented 5 years ago

@memsharded, agreed there is a risk, but this change at least makes it solvable. It may be possible to determine if multiple configurations are using the same installation directory and issue a warning.

Yes, sure, I am thinking on stating it clear in the docs, and that was exactly what I was thinking of, a warning that will tell if you are reusing same folder for different configs

SSE4 commented 5 years ago

@jodylittle I think it should be already outputted, to the Debug -> Windows -> Output, and select Conan

SSE4 commented 5 years ago

for the macro evaluation, we just rely on how Visual Studio evaluates them, and it turns out as it just evaluates undefined macro definitions into empty strings, without any error indication

jodylittle commented 5 years ago

Thanks, I somehow missed that the full Conan command was written out to the console.

I was testing the macro and evaluation and for me it would throw an exception. I was using generalSettings.GetEvaluatedPropertyValue and it would throw a ProjectException like this: Exception

You can see my change in a fork here.

SSE4 commented 5 years ago

@jodylittle cool! I'll integrate your changes then, as it's useful addition

SSE4 commented 5 years ago

@jodylittle after closer look, it throws an exception because you're using ConfigurationGeneral to evaluate properties. it also throws the same exception for ProjectDir and SolutionDir, for example. that's logical, as General tab contains only few properties, like OutDir or IntDir. to evaluate all properties, I am just using VCConfiguration object itself, which is able to recognize all properties, include ProjectDir and SolutionDir. and both methods (GetEvaluatedPropertyValue and Evaluate) resolve unknown properties to the empty string. so I guess the current behaviour is accurate.

jodylittle commented 5 years ago

@SSE4, that makes sense. Thanks for the info.

jgsogo commented 5 years ago

I've added a line to the output to write the path to the property sheet added to the project (https://github.com/conan-io/conan-vs-extension/pull/134/commits/4908b8bb741a33db4f3fa35dbd03161c691067fc). Also I've documented the option with an advise to use VS dynamic dirs like $(Configuration),...

There is still one flaw associated with this, but not included in PR #134 (I'll move this argument to a new issue): we are checking if the property file is already added to the project to avoid duplication using the path to the property sheet (#128 pending of investigation), now we allow the user to change the path so potentially we may end with two files added to the project. We would need a better way to identify these files generated by Conan and remove all of them before adding the right one to the project.