Skip to content

Check if references file exists on disk before installing into project file - #2224

Merged
forki merged 2 commits into
fsprojects:masterfrom
bchavez:master
Apr 10, 2017
Merged

forki merged 2 commits into
fsprojects:masterfrom
bchavez:master

Conversation

@bchavez

@bchavez bchavez commented Apr 9, 2017 •

Copy link
Copy Markdown
Contributor

Check if paket.references file exists on disk before allowing paket to blindly install/modify (csproj) project files with <Import>. If paket.references is missing from a project directory, paket should not touch any project files.

Also, according to @forki:

If you remove the paket.references file from the dotnetcore project folder
then it should ignore the csproj.

This is currently not true as there is no check here to exclude project files (csprojs) from the FindAllProjects enumeration. Additionally, this particular issue has caused some drama in issue #2029.

This PR resolves the issue and makes sure that paket.references exists on disk before paket does anything to MSBuild project files.

Also resolves #2220 by providing a way to opt-out of the <Import> mechanism.

Thanks,
Brian

🍫 🍪 🍭 Ronald Jenkees - Stay Crunchy

@bchavez

bchavez commented Apr 9, 2017

Copy link
Copy Markdown
Contributor Author

And, in addition, got your Travis CI build working again. 👍

🚶 😎 "So don't delay... act now supplies are running out..."

@forki

forki commented Apr 10, 2017

Copy link
Copy Markdown
Member

@agross do you remember how we defined that case?

If a csproj doesn't have a corresponding references file - should treat it as empy or do we ignore the project?

@agross

agross commented Apr 10, 2017

Copy link
Copy Markdown
Contributor

@forki If a ??proj doesn't have a paket.references or the specific proj.??proj.references (not sure about the naming convention right now) file next to it, how would paket decide what to install in the first place. So I think yes, we need to ignore it.

@forki

forki commented Apr 10, 2017

Copy link
Copy Markdown
Member

how would paket decide what to install in the first place

currently it's creating an empty references file in memory. So it does not install anything - but it touches it since it wants to clean.

@bchavez

bchavez commented Apr 10, 2017 •

Copy link
Copy Markdown
Contributor Author

So it does not install anything - but it touches it since it wants to clean.

Yes it does install something in the .??proj file, this right here:

<Import Project="..\.paket\Paket.Restore.targets" />

Even when there is no paket.references. This is the problem in #2029 and #2220 ❗❗❗

@forki

forki commented Apr 10, 2017

Copy link
Copy Markdown
Member

I wished you would not see there targets as a problem. It's the only solution to get netcore working. They might not work for you yet. but it's the important part that needs fixing.

@forki
forki merged commit 374e321 into fsprojects:master Apr 10, 2017
bchavez added a commit to bchavez/Bogus that referenced this pull request Apr 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Visual Studio 2017 new csproj and Paket.Restore.targets import

3 participants