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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 2 commits into from
Apr 10, 2017
Merged

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

merged 2 commits into from
Apr 10, 2017

Conversation

bchavez
Copy link
Contributor

@bchavez bchavez commented Apr 9, 2017 •
edited
Loading

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
Copy link
Contributor Author

bchavez commented Apr 9, 2017

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

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

@forki
Copy link
Member

forki commented Apr 10, 2017

@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
Copy link
Contributor

agross commented Apr 10, 2017

@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
Copy link
Member

forki commented Apr 10, 2017

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
Copy link
Contributor Author

bchavez commented Apr 10, 2017 •
edited
Loading

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
Copy link
Member

forki commented Apr 10, 2017

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
@bchavez
Reverting paket.dependencies to root source folder. Paket no longer i…
3af5015 
…nterferes with csproj: fsprojects/Paket#2224
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Visual Studio 2017 new csproj and Paket.Restore.targets import
3 participants
@bchavez @forki @agross