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

bcl.targets is not compatible with Paket #604

Closed
isaacabraham opened this issue Feb 13, 2015 · 22 comments
Closed

bcl.targets is not compatible with Paket #604

isaacabraham opened this issue Feb 13, 2015 · 22 comments

Comments

@isaacabraham
Copy link
Contributor

If you reference any of the BCL nuget packages, they add a target (Microsoft.Bcl.Build.targets) to the project file. This in turn tries to validate package references by looking for, you guessed it - packages.config.

Not sure what the solution to this is but it leads to a build error and associated warnings.

@forki forki added the bug label Feb 13, 2015
@forki
Copy link
Member

forki commented Feb 13, 2015

what about backlisting these?

@mavnn
Copy link
Contributor

mavnn commented Feb 13, 2015

I have never had anything but pain from bcl.targets, and I've never seen a failure caused by removing them.

Even with Nuget they tend to break if you configure a non-standard package directory.

@forki
Copy link
Member

forki commented Feb 13, 2015

would love to see a pull request which blacklists these targets files.

@isaacabraham
Copy link
Contributor Author

@mavnn - they confuse the hell out of me too.
@forki - I assume that we're seeing them now that targets support is in paket. Yes a blacklist a good idea. Maybe the only idea aside from manually rewriting the targets file to not look for packages.config. But I don't think that we want to go down that route!

A bit concerned about where this path will lead us down though.

@forki
Copy link
Member

forki commented Feb 13, 2015

@dedale
Copy link
Contributor

dedale commented Feb 15, 2015

I cannot reproduce the build failures under VS. The Bcl targets tells me that adding the following property in the project would skip nuget validation:

<SkipValidatePackageReferences>true</SkipValidatePackageReferences>

@damageboy
Copy link

@forki I'll take a stab at it.
I assume some sort of and embedded resource like a blacklist.txt file that is part of paket is acceptable?

@ctaggart
Copy link
Contributor

Can I get an option to turn off this feature (no automatic .props or .targets)? Like NuGet content support, this is a feature that I would like to turn off. I was very excited about it when came out in NuGet 2.5, but it doesn't work very well. The Microsoft.Bcl.Build.targets are preventing me from upgrading paket.

@ctaggart
Copy link
Contributor

How about a feature to opt-out of automatic import of targets with syntax from #574?

nuget Foobar 1.2.3 framework: net20 net35, autoimport: false

@damageboy
Copy link

I leave it to @forki to decide.

I know the ms BCL is a special kind of crap in this respect. I haven't seen anything else that bothers me that much... I don't think it's a bad idea to disable autoimport though, could be useful for more scenarios, but unlike what @ctaggart suggested, I think it should be a paket.references things, just like copy_local is

@isaacabraham
Copy link
Contributor Author

Note that you will need targets for things like XUnit test runner in future (that's what got this whole .targets malarky started). But I agree - this BCL stuff is just a world of pain.

@isaacabraham
Copy link
Contributor Author

@damageboy I wouldn't bother with an embedded text file personally - just make an F# list of strings for the blacklist (perhaps as it's own .fs file). F# is lightweight enough to not have fluff get in the way, and we're not going to do "drop in" replacements of the file.

@forki
Copy link
Member

forki commented Feb 16, 2015

Please start by extending the blacklist. I need to think about a good way
to disable the target feature, but I agree we need a switch for that.

But I think it's safe to say that everyone wants to disable the bcl stuff
so blacklisting is still a good idea.
On Feb 16, 2015 10:21 PM, "Isaac Abraham" [email protected] wrote:

@damageboy https://github.com/damageboy I wouldn't bother with an
embedded text file personally - just make an F# list of strings for the
blacklist (perhaps as it's own .fs file). F# is lightweight enough to not
have fluff get in the way, and we're not going to do "drop in" replacements
of the file.


Reply to this email directly or view it on GitHub
#604 (comment).

@ascjones
Copy link
Contributor

Deleted previous comment - I hadn't refreshed the page from earlier to see that this issue is closed!

I'd done a quick fix with a blacklist but obviously not needed now.

@forki
Copy link
Member

forki commented Feb 17, 2015

The issue is not closed. We still need a fix, but we have a workaround. You can disable the targets - see #615

@ascjones
Copy link
Contributor

Doh my mistake! Saw the Closed icon for the linked issue

In that case here is a quick fix for that with a blacklist I did to get it working for us locally:

geniussportsgroup@a0ca57b

If that's any good and assuming @damageboy hasn't got something better in store then I can submit it as a PR.

@forki
Copy link
Member

forki commented Feb 17, 2015

@ascjones
Copy link
Contributor

Actually that might do the trick for us, more practical than adding to all the individual references files. Especially when dealing with the Bcl.Build which is a transitive dependency.


Sent from Mailbox

On Tue, Feb 17, 2015 at 8:33 PM, Steffen Forkmann
[email protected] wrote:

I also added http://fsprojects.github.io/Paket/dependencies-file.html#import_targets-settings

Reply to this email directly or view it on GitHub:
#604 (comment)

@forki
Copy link
Member

forki commented Feb 17, 2015

I still think it's a very good idea to blacklist since the package is
broken for everyone who is using paket.
On Feb 17, 2015 10:49 PM, "Andrew Jones" [email protected] wrote:

Actually that might do the trick for us, more practical than adding to all
the individual references files. Especially when dealing with the Bcl.Build
which is a transitive dependency.


Sent from Mailbox

On Tue, Feb 17, 2015 at 8:33 PM, Steffen Forkmann
[email protected] wrote:

I also added

http://fsprojects.github.io/Paket/dependencies-file.html#import_targets-settings

Reply to this email directly or view it on GitHub:
#604 (comment)


Reply to this email directly or view it on GitHub
#604 (comment).

@forki
Copy link
Member

forki commented Feb 17, 2015

Also keep an eye on #587

@ascjones
Copy link
Contributor

Cool just created a PR, might want to check I've done it right.

@forki forki closed this as completed Feb 18, 2015
@forki
Copy link
Member

forki commented Feb 18, 2015

so it's blacklisted and a streamlined ìmport_targets switch (see #587)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants