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

.Net Framework is pulling in .Net Standard libraries #1457

Closed
john2014 opened this Issue Feb 24, 2017 · 27 comments

Comments

Projects
None yet
6 participants
@john2014

john2014 commented Feb 24, 2017

I am currently using .NET 4.6.2 framework. When I install Npgsql 3.1.9 (or below) from Nuget (via Visual studio 2015 community edition), it is correctly pulling in only the required libraries for Npgsql.

However when I install Npgsql 3.2.0 or 3.2.1 from Nuget (via Visual Studio 2015), it is also installing .Net Standard Library 1.6.1 as well as HUGE number of libraries which are dependent on it (System.Collection, System.IO, System.Net.Http, System.Runtime, System.Threading, System.Xml, etc).

It should not be pulling in all these libraries, since it already exists in the full .NET Framework 4.6.2.

@john2014 john2014 changed the title from .Net Frramework is pulling in .Net Standard libraries to .Net Framework is pulling in .Net Standard libraries Feb 24, 2017

@roji

This comment has been minimized.

Show comment
Hide comment
@roji

roji Feb 25, 2017

Member

Duplicate of #1430.

In a nutshell, the .NET Standard dependency comes via Microsoft.Extensions.Logging. It's completely fine for a .NET Framework library to depend on .NET Standard - that's part of the point of having the standard.

It's true that there are a lot of nuget dependencies, but when you actually build your application there are no actual DLLs or assemblies, since in .NET Framework the functionality is already available in your GAC. So there isn't any actual effect on your application, except for the build-time addition of nugets. Even that nuget bloat will be gone with .NET Standard 2.0, which will be released later this year.

Member

roji commented Feb 25, 2017

Duplicate of #1430.

In a nutshell, the .NET Standard dependency comes via Microsoft.Extensions.Logging. It's completely fine for a .NET Framework library to depend on .NET Standard - that's part of the point of having the standard.

It's true that there are a lot of nuget dependencies, but when you actually build your application there are no actual DLLs or assemblies, since in .NET Framework the functionality is already available in your GAC. So there isn't any actual effect on your application, except for the build-time addition of nugets. Even that nuget bloat will be gone with .NET Standard 2.0, which will be released later this year.

@roji roji closed this Feb 25, 2017

@roji roji added the duplicate label Feb 25, 2017

@mattzink

This comment has been minimized.

Show comment
Hide comment
@mattzink

mattzink Feb 28, 2017

This should be re-opened. For a new .NET 4.6.1 project, installing only npgsql nuget package adds 27(!) files that are copied on build and required to run. This seems ridiculous just to get a logging abstraction.

Microsoft.Extensions.Logging.Abstractions.xml"
Microsoft.Extensions.Logging.dll"
Microsoft.Extensions.Logging.xml"
Microsoft.Win32.Primitives.dll"
Npgsql.dll"
Microsoft.Extensions.DependencyInjection.Abstractions.dll"
Microsoft.Extensions.DependencyInjection.Abstractions.xml"
Microsoft.Extensions.Logging.Abstractions.dll"
System.IO.Compression.dll"
System.IO.Compression.ZipFile.dll"
System.IO.FileSystem.dll"
System.IO.FileSystem.Primitives.dll"
System.Net.Http.dll"
System.Net.Sockets.dll"
System.Runtime.InteropServices.RuntimeInformation.dll"
System.Security.Cryptography.Algorithms.dll"
System.Security.Cryptography.Encoding.dll"
System.Security.Cryptography.Primitives.dll"
System.Security.Cryptography.X509Certificates.dll"
System.Threading.Tasks.Extensions.dll"
System.Threading.Tasks.Extensions.xml"
System.Xml.ReaderWriter.dll"
System.AppContext.dll"
System.Console.dll"
System.Diagnostics.DiagnosticSource.dll"
System.Diagnostics.DiagnosticSource.xml"
System.Globalization.Calendars.dll"

mattzink commented Feb 28, 2017

This should be re-opened. For a new .NET 4.6.1 project, installing only npgsql nuget package adds 27(!) files that are copied on build and required to run. This seems ridiculous just to get a logging abstraction.

Microsoft.Extensions.Logging.Abstractions.xml"
Microsoft.Extensions.Logging.dll"
Microsoft.Extensions.Logging.xml"
Microsoft.Win32.Primitives.dll"
Npgsql.dll"
Microsoft.Extensions.DependencyInjection.Abstractions.dll"
Microsoft.Extensions.DependencyInjection.Abstractions.xml"
Microsoft.Extensions.Logging.Abstractions.dll"
System.IO.Compression.dll"
System.IO.Compression.ZipFile.dll"
System.IO.FileSystem.dll"
System.IO.FileSystem.Primitives.dll"
System.Net.Http.dll"
System.Net.Sockets.dll"
System.Runtime.InteropServices.RuntimeInformation.dll"
System.Security.Cryptography.Algorithms.dll"
System.Security.Cryptography.Encoding.dll"
System.Security.Cryptography.Primitives.dll"
System.Security.Cryptography.X509Certificates.dll"
System.Threading.Tasks.Extensions.dll"
System.Threading.Tasks.Extensions.xml"
System.Xml.ReaderWriter.dll"
System.AppContext.dll"
System.Console.dll"
System.Diagnostics.DiagnosticSource.dll"
System.Diagnostics.DiagnosticSource.xml"
System.Globalization.Calendars.dll"

@roji

This comment has been minimized.

Show comment
Hide comment
@roji

roji Mar 1, 2017

Member

@mattzink, I really do sympathize - I'm really not a big fan of these extra packages.

But seriously, the combined size of all these is 865KB on my machine - less than one MB; this is because all they do is contain type forwarders to the .NET Framework implementation. Aside from aversion to bloat as a principle (which I do understand and I identify with), I can't see any concrete problem this can raise.

What's more, as posted above, once .NET Standard 2.0 comes out, even this inconsequential bloat will be gone.

I'm really open to having a conversation about this, and do think that LibLog can be a very worthy alternative to Microsoft.Extensions.Logging (which is bringing in these dependencies), if it solves its performance issues. But there need to be actual arguments beyond "I don't like the extra 865KB bloat".

Member

roji commented Mar 1, 2017

@mattzink, I really do sympathize - I'm really not a big fan of these extra packages.

But seriously, the combined size of all these is 865KB on my machine - less than one MB; this is because all they do is contain type forwarders to the .NET Framework implementation. Aside from aversion to bloat as a principle (which I do understand and I identify with), I can't see any concrete problem this can raise.

What's more, as posted above, once .NET Standard 2.0 comes out, even this inconsequential bloat will be gone.

I'm really open to having a conversation about this, and do think that LibLog can be a very worthy alternative to Microsoft.Extensions.Logging (which is bringing in these dependencies), if it solves its performance issues. But there need to be actual arguments beyond "I don't like the extra 865KB bloat".

@mattzink

This comment has been minimized.

Show comment
Hide comment
@mattzink

mattzink Mar 2, 2017

@roji, I see the bind this puts you (and anybody else trying to reference .NET standard currently) in and appreciate that. The issue isn't the 865kb, it's that this becomes a transitive dependency for users of our package, and users of their packages, and so on. This is just a fact of life when it's really necessary, but in this case since you are only using it to get a logging abstraction, could I ask you to please consider using an alternative solution until .NET Standard 2.0 is released (currently planned for Q3)?

mattzink commented Mar 2, 2017

@roji, I see the bind this puts you (and anybody else trying to reference .NET standard currently) in and appreciate that. The issue isn't the 865kb, it's that this becomes a transitive dependency for users of our package, and users of their packages, and so on. This is just a fact of life when it's really necessary, but in this case since you are only using it to get a logging abstraction, could I ask you to please consider using an alternative solution until .NET Standard 2.0 is released (currently planned for Q3)?

@roji

This comment has been minimized.

Show comment
Hide comment
@roji

roji Mar 2, 2017

Member

@mattzink, can you explain why the added transitive dependency is an actual problem?

Member

roji commented Mar 2, 2017

@mattzink, can you explain why the added transitive dependency is an actual problem?

@roji

This comment has been minimized.

Show comment
Hide comment
@roji

roji Mar 2, 2017

Member

What I meant to say, is that libraries very frequently come with transitive dependencies. If you decide to use something like Entity Framework, you will be pulling in many more dependencies that what Npgsql just added - this is just how things work. The fact that a dependency is added (transitively or not) doesn't seem like an issue by itself...

Member

roji commented Mar 2, 2017

What I meant to say, is that libraries very frequently come with transitive dependencies. If you decide to use something like Entity Framework, you will be pulling in many more dependencies that what Npgsql just added - this is just how things work. The fact that a dependency is added (transitively or not) doesn't seem like an issue by itself...

@mattzink

This comment has been minimized.

Show comment
Hide comment
@mattzink

mattzink Mar 2, 2017

As I said above, some dependencies are necessary, but they have a cost which should be considered. In my case, we will now need to also update our Linux RPM packages to include all these files (which may change at any point in the future if any part of the package dependency chain is updated), we have many other assemblies in the same bin directory that will now need binding redirects (since this pulls in a different version of libraries than those that are in the GAC), and we need to vet/include the additional licenses, etc. In my opinion, this burden to users is not worth a simple abstraction, whose interfaces could be copied internally to npgsql until .NET standard 2.0 is released and ubiquitous. This is obviously a subjective subject, so I don't have any have anything more concrete to say other than the pain this causes me seems to be more than what you are gaining :-)

mattzink commented Mar 2, 2017

As I said above, some dependencies are necessary, but they have a cost which should be considered. In my case, we will now need to also update our Linux RPM packages to include all these files (which may change at any point in the future if any part of the package dependency chain is updated), we have many other assemblies in the same bin directory that will now need binding redirects (since this pulls in a different version of libraries than those that are in the GAC), and we need to vet/include the additional licenses, etc. In my opinion, this burden to users is not worth a simple abstraction, whose interfaces could be copied internally to npgsql until .NET standard 2.0 is released and ubiquitous. This is obviously a subjective subject, so I don't have any have anything more concrete to say other than the pain this causes me seems to be more than what you are gaining :-)

@roji

This comment has been minimized.

Show comment
Hide comment
@roji

roji Mar 3, 2017

Member

@mattzink and everyone, due to the number of complaints I'll rethink about this (you guys are the users after all). I'm not sure yet whether we would move back to the custom Npgsql API of 3.0/3.1, or to LibLog, or even if such a big switch should happen in a patch-level 3.2 release (it doesn't, but...).

So stand by...

Member

roji commented Mar 3, 2017

@mattzink and everyone, due to the number of complaints I'll rethink about this (you guys are the users after all). I'm not sure yet whether we would move back to the custom Npgsql API of 3.0/3.1, or to LibLog, or even if such a big switch should happen in a patch-level 3.2 release (it doesn't, but...).

So stand by...

@rwasef1830

This comment has been minimized.

Show comment
Hide comment
@rwasef1830

rwasef1830 Mar 3, 2017

Contributor

My opinion would be personally to leave it as-is. All this will disappear with .net standard 2.0
But... if people insist, a very thin custom interface would do. And then at startup we could search for ms logging and hook that up automatically with a ms logging implementation and expose an api to let users replace it with what they want (and they implement the api).

Contributor

rwasef1830 commented Mar 3, 2017

My opinion would be personally to leave it as-is. All this will disappear with .net standard 2.0
But... if people insist, a very thin custom interface would do. And then at startup we could search for ms logging and hook that up automatically with a ms logging implementation and expose an api to let users replace it with what they want (and they implement the api).

@roji

This comment has been minimized.

Show comment
Hide comment
@roji

roji Mar 3, 2017

Member

@rwasef1830 thanks for your opinion. Even though a lot of the bloat disappears with netstandard20, there's still the dependency on Microsoft.Extensions.Logging, and through it, on Microsoft.DependencyInjection.Abstractions (which at least in theory can lead to dependency hell if Npgsql is used in an application which depends on other versions of these nugets).

The "very thin custom interface" which you propose is more or less what we had in 3.0/3.1. I only recently learned of the existence of LibLog, which is superior version of that idea - you drop it inside your library project, and it comes with built-in support for the major logging packages, using reflection to access them (to avoid the reference). It can even auto-detect what logging package is already installed and use that, obviating manual/explicit setup. IMHO this is the best solution to the library logging problem - no dependencies and wide support for packages - if I'd known about it before releasing 3.2 I'd probably have gone with it.

The only current issues with LibLog are no netstandard support and especially some performance issues (see damianh/LibLog#129). I might contribute some PRs there to fix these issues and then just switch to it.

Member

roji commented Mar 3, 2017

@rwasef1830 thanks for your opinion. Even though a lot of the bloat disappears with netstandard20, there's still the dependency on Microsoft.Extensions.Logging, and through it, on Microsoft.DependencyInjection.Abstractions (which at least in theory can lead to dependency hell if Npgsql is used in an application which depends on other versions of these nugets).

The "very thin custom interface" which you propose is more or less what we had in 3.0/3.1. I only recently learned of the existence of LibLog, which is superior version of that idea - you drop it inside your library project, and it comes with built-in support for the major logging packages, using reflection to access them (to avoid the reference). It can even auto-detect what logging package is already installed and use that, obviating manual/explicit setup. IMHO this is the best solution to the library logging problem - no dependencies and wide support for packages - if I'd known about it before releasing 3.2 I'd probably have gone with it.

The only current issues with LibLog are no netstandard support and especially some performance issues (see damianh/LibLog#129). I might contribute some PRs there to fix these issues and then just switch to it.

@john2014

This comment has been minimized.

Show comment
Hide comment
@john2014

john2014 Mar 4, 2017

Few things I would like to add:

1.If Roji moves logging to either LibLog or internal logging (which existed in 3.0), would you move it back to Microsoft logging when .net standard 20 is released around July 2017?
2. If you change the logging layer (to eg Liblog), I think it would be better to release a version 3.3 (instead of 3.2.x).
3. I saw Roji open a bug on Microsoft Github page about multiple dependency. They mentioned that all of this would be rolled into one file in .net standard 2.0. The only way that would be possible is if they copy/paste function from different files and move it into one file. This would mean if there is a bug in one function, they would now have to fix it in multiple files where it was copy/pasted into.
4. In Npgsql.org website, there is a header link called "API Documentation". However the tag does not have href attribute. Hence nothing happens when you click on it.
5. Also most people may not be using the logging functionality (especially for small sites or internal applications). Would it be helpful to these people to release Npgslq version with out any logging abstractions?

And finally thank you for creating such a great library 😄

john2014 commented Mar 4, 2017

Few things I would like to add:

1.If Roji moves logging to either LibLog or internal logging (which existed in 3.0), would you move it back to Microsoft logging when .net standard 20 is released around July 2017?
2. If you change the logging layer (to eg Liblog), I think it would be better to release a version 3.3 (instead of 3.2.x).
3. I saw Roji open a bug on Microsoft Github page about multiple dependency. They mentioned that all of this would be rolled into one file in .net standard 2.0. The only way that would be possible is if they copy/paste function from different files and move it into one file. This would mean if there is a bug in one function, they would now have to fix it in multiple files where it was copy/pasted into.
4. In Npgsql.org website, there is a header link called "API Documentation". However the tag does not have href attribute. Hence nothing happens when you click on it.
5. Also most people may not be using the logging functionality (especially for small sites or internal applications). Would it be helpful to these people to release Npgslq version with out any logging abstractions?

And finally thank you for creating such a great library 😄

@roji

This comment has been minimized.

Show comment
Hide comment
@roji

roji Mar 4, 2017

Member

1.If Roji moves logging to either LibLog or internal logging (which existed in 3.0), would you move it back to Microsoft logging when .net standard 20 is released around July 2017?

My current preference is to move to LibLog rather to back to the custom logging abstraction that existed in 3.0/3.1. I think that if that move occurs, there's no real reason to move back to Microsoft logging even after netstandard20 is released - LibLog is a perfectly fine approach that would have zero dependencies, where Microsoft's abstraction would still bring in the logging abstraction dependency and the dependency injection abstractions.

But I'm open to thoughts/opinions and definitely am not sure yet.

  1. If you change the logging layer (to eg liblog), I think it would be better to release a version 3.3 (instead of 3.2.x).

I also dislike making a change like this in a patch-level release. However, as you yourself correctly pointed out in point 5 below, logging is a pretty minor part of Npgsql's API surface, and is likely very little-used. Also, there's very little risk of introducing bugs - it's just a logging abstraction change. For these reasons it makes more sense for me to release this in a patch release and avoid the confusion associated with a 3.3. release so soon after the 3.2.

  1. I saw Roji open a bug on Microsoft github page about multiple dependency. They mentioned that all of this would be rolled into one file in .net standard 2.0. The only way that would be possible is if they copy/paste function from different files and move it into one file. This would mean if there is a bug in one function, they would now have to fix it in multiple files where it was copy/pasted into.

I'm not sure I understand this one... You're right that after netstandard20 is out, they'll have both netstandard20 and netstandard16 to maintain, with the same functions existing in different files. But this is more of an internal Microsoft issue etc.

Note that this already exists to a certain exists - .NET Framework and .NET Core share lots of functions, and I'm guessing that when a bug is found they have to fix in both etc. Regardless, again, not sure what this implies for us.

  1. In Npgsql.com website, there is a header link called "API Documentation". However the tag does not have href attribute. Hence nothing happens when you click on it.

Can you please open a separate issue for this?

  1. Also most people may not be using the logging functionality (especially for small sites or internal applications). Would it be helpful to these people to release Npgslq version with out any logging abstractions?

I agree with the observation that it's probably rare for people to actually use the logging functionality, which is why a no-dependency approach (like LibLog) is very appealing. It has all the advantages (i.e. transparently allowing you use what logging library you want) without the disadvantages. So I don't really see a reason to have a version without any abstraction.

And finally thank you for creating such a great library 😄

Thanks for the positive feedback :)

Member

roji commented Mar 4, 2017

1.If Roji moves logging to either LibLog or internal logging (which existed in 3.0), would you move it back to Microsoft logging when .net standard 20 is released around July 2017?

My current preference is to move to LibLog rather to back to the custom logging abstraction that existed in 3.0/3.1. I think that if that move occurs, there's no real reason to move back to Microsoft logging even after netstandard20 is released - LibLog is a perfectly fine approach that would have zero dependencies, where Microsoft's abstraction would still bring in the logging abstraction dependency and the dependency injection abstractions.

But I'm open to thoughts/opinions and definitely am not sure yet.

  1. If you change the logging layer (to eg liblog), I think it would be better to release a version 3.3 (instead of 3.2.x).

I also dislike making a change like this in a patch-level release. However, as you yourself correctly pointed out in point 5 below, logging is a pretty minor part of Npgsql's API surface, and is likely very little-used. Also, there's very little risk of introducing bugs - it's just a logging abstraction change. For these reasons it makes more sense for me to release this in a patch release and avoid the confusion associated with a 3.3. release so soon after the 3.2.

  1. I saw Roji open a bug on Microsoft github page about multiple dependency. They mentioned that all of this would be rolled into one file in .net standard 2.0. The only way that would be possible is if they copy/paste function from different files and move it into one file. This would mean if there is a bug in one function, they would now have to fix it in multiple files where it was copy/pasted into.

I'm not sure I understand this one... You're right that after netstandard20 is out, they'll have both netstandard20 and netstandard16 to maintain, with the same functions existing in different files. But this is more of an internal Microsoft issue etc.

Note that this already exists to a certain exists - .NET Framework and .NET Core share lots of functions, and I'm guessing that when a bug is found they have to fix in both etc. Regardless, again, not sure what this implies for us.

  1. In Npgsql.com website, there is a header link called "API Documentation". However the tag does not have href attribute. Hence nothing happens when you click on it.

Can you please open a separate issue for this?

  1. Also most people may not be using the logging functionality (especially for small sites or internal applications). Would it be helpful to these people to release Npgslq version with out any logging abstractions?

I agree with the observation that it's probably rare for people to actually use the logging functionality, which is why a no-dependency approach (like LibLog) is very appealing. It has all the advantages (i.e. transparently allowing you use what logging library you want) without the disadvantages. So I don't really see a reason to have a version without any abstraction.

And finally thank you for creating such a great library 😄

Thanks for the positive feedback :)

@john2014

This comment has been minimized.

Show comment
Hide comment
@john2014

john2014 Mar 4, 2017

  1. In http://www.npgsql.org/doc/migration/3.2.html you mentioned the breaking change as switching logging abstraction to Microsoft. Hence it may confuse (some) people if you change logging abstraction again in 3.2.2 to something else.

In rare cases (I think), it is ok to release 3.3 soon after 3.2 (jquery has done it few times because of critical bugs or fixes which had high impact).
3. This doesn't (directly) affect npgsql. Just something I wondered. Because if they are merging so many different functions (from 27 files) into one file just for logging abstraction, they would be doing the same for other functionalities (apart from logging). So for Microsoft, it could be very hard to keep track of things. :)
4. I have opened separate issue #1473

  1. For LibLog, I was looking at release/changelog: https://github.com/damianh/LibLog/releases

It seems the author may not be actively working on it. Since LibLog is MIT license, maybe you can fork it and add your PR (if its faster)? Though it is always a good idea to create one library (instead of multiple forks).

john2014 commented Mar 4, 2017

  1. In http://www.npgsql.org/doc/migration/3.2.html you mentioned the breaking change as switching logging abstraction to Microsoft. Hence it may confuse (some) people if you change logging abstraction again in 3.2.2 to something else.

In rare cases (I think), it is ok to release 3.3 soon after 3.2 (jquery has done it few times because of critical bugs or fixes which had high impact).
3. This doesn't (directly) affect npgsql. Just something I wondered. Because if they are merging so many different functions (from 27 files) into one file just for logging abstraction, they would be doing the same for other functionalities (apart from logging). So for Microsoft, it could be very hard to keep track of things. :)
4. I have opened separate issue #1473

  1. For LibLog, I was looking at release/changelog: https://github.com/damianh/LibLog/releases

It seems the author may not be actively working on it. Since LibLog is MIT license, maybe you can fork it and add your PR (if its faster)? Though it is always a good idea to create one library (instead of multiple forks).

@john2014

This comment has been minimized.

Show comment
Hide comment
@john2014

john2014 Mar 4, 2017

I forgot to add:

Here are some of the advantages of library without logging abstractions:

  1. Smaller size of Npgsql (since it doesnt include unnecessary code).
  2. The code does not have to check (on every?) call whether logging is enabled or not. If yes, then do the logging. Hence this would speed up code execution (especially on site serving millions of people daily).
  3. A bug in LibLog would not affect Npgsql. Hence an attacker cannot use bug in LibLog to pass on attacks to Npgsql
  4. If a bug is found in LibLog, Npgsql (without LibLog) wont need to immediately update their code to latest version.

john2014 commented Mar 4, 2017

I forgot to add:

Here are some of the advantages of library without logging abstractions:

  1. Smaller size of Npgsql (since it doesnt include unnecessary code).
  2. The code does not have to check (on every?) call whether logging is enabled or not. If yes, then do the logging. Hence this would speed up code execution (especially on site serving millions of people daily).
  3. A bug in LibLog would not affect Npgsql. Hence an attacker cannot use bug in LibLog to pass on attacks to Npgsql
  4. If a bug is found in LibLog, Npgsql (without LibLog) wont need to immediately update their code to latest version.
@john2014

This comment has been minimized.

Show comment
Hide comment
@john2014

john2014 Mar 4, 2017

Also since you are mimicking MS SQL Server driver, it may be helpful to change namespace from "Using Npgsql" to "using System.Data.Npgsql".

john2014 commented Mar 4, 2017

Also since you are mimicking MS SQL Server driver, it may be helpful to change namespace from "Using Npgsql" to "using System.Data.Npgsql".

@roji

This comment has been minimized.

Show comment
Hide comment
@roji

roji Mar 4, 2017

Member

@john2014,

For LibLog, I was looking at release/changelog: https://github.com/damianh/LibLog/releases

It seems the author may not be actively working on it. Since LibLog is MIT license, maybe you can fork it and add your PR (if its faster)? Though it is always a good idea to create one library (instead of multiple forks).

That's true, I reached out to the author to see how they see things. I'd really prefer not to end up maintaining a logging abstraction package in addition to Npgsql. Regardless, the logging situation in Npgsql isn't going to change right away (i.e. for 3.2.2), so we can wait and see.

Here are some of the advantages of library without logging abstractions:

Smaller size of Npgsql (since it doesnt include unnecessary code).

That's pretty negligible... Also, you could say the same about many other Npgsql features - how many people actually use COPY (binary input)? SSL/TLS? It doesn't make sense to start stripping those out etc.

The code does not have to check (on every?) call whether logging is enabled or not. If yes, then do the logging. Hence this would speed up code execution (especially on site serving millions of people daily).

That's somewhat true, and one of my current gripes with LibLog is that the price of a disabled log call is too high - I'm talking about when Npgsql logs in debug but debug isn't enabled. In other words, while it's somewhat OK for enabled logging calls to be slow (since they're likely to be disabled anyway), it's very important for disabled calls to be as fast as possible.

But let's put things into perspective - Npgsql is a library that executes network operations over TCP/IP with remote databases... The overhead of these logging calls is really negligible.

A bug in LibLog would not affect Npgsql. Hence an attacker cannot use bug in LibLog to pass on attacks to Npgsql

There's always the possibility of bugs... It's particularly hard to imagine meaningful/dangerous bugs in a thin logging abstraction such as LibLog. Regardless, I'm not sure what "attacking Npgsql" means... Both Npgsql and LibLog reside in the same process which the user owns, why anyone would need to use LibLog to attack Npgsql is beyond me.

If a bug is found in LibLog, Npgsql (without LibLog) wont need to immediately update their code to latest version.

Again, the possibility of bugs in itself isn't a reason to avoid all dependencies in every scenario. LibLog in particular is a very non-critical dependency that is by default "off", so a bug isn't likely to affect most people. Npgsql also has a pretty extensive regression test suite, so any meaningful bugs should get caught before releasing a version.

Member

roji commented Mar 4, 2017

@john2014,

For LibLog, I was looking at release/changelog: https://github.com/damianh/LibLog/releases

It seems the author may not be actively working on it. Since LibLog is MIT license, maybe you can fork it and add your PR (if its faster)? Though it is always a good idea to create one library (instead of multiple forks).

That's true, I reached out to the author to see how they see things. I'd really prefer not to end up maintaining a logging abstraction package in addition to Npgsql. Regardless, the logging situation in Npgsql isn't going to change right away (i.e. for 3.2.2), so we can wait and see.

Here are some of the advantages of library without logging abstractions:

Smaller size of Npgsql (since it doesnt include unnecessary code).

That's pretty negligible... Also, you could say the same about many other Npgsql features - how many people actually use COPY (binary input)? SSL/TLS? It doesn't make sense to start stripping those out etc.

The code does not have to check (on every?) call whether logging is enabled or not. If yes, then do the logging. Hence this would speed up code execution (especially on site serving millions of people daily).

That's somewhat true, and one of my current gripes with LibLog is that the price of a disabled log call is too high - I'm talking about when Npgsql logs in debug but debug isn't enabled. In other words, while it's somewhat OK for enabled logging calls to be slow (since they're likely to be disabled anyway), it's very important for disabled calls to be as fast as possible.

But let's put things into perspective - Npgsql is a library that executes network operations over TCP/IP with remote databases... The overhead of these logging calls is really negligible.

A bug in LibLog would not affect Npgsql. Hence an attacker cannot use bug in LibLog to pass on attacks to Npgsql

There's always the possibility of bugs... It's particularly hard to imagine meaningful/dangerous bugs in a thin logging abstraction such as LibLog. Regardless, I'm not sure what "attacking Npgsql" means... Both Npgsql and LibLog reside in the same process which the user owns, why anyone would need to use LibLog to attack Npgsql is beyond me.

If a bug is found in LibLog, Npgsql (without LibLog) wont need to immediately update their code to latest version.

Again, the possibility of bugs in itself isn't a reason to avoid all dependencies in every scenario. LibLog in particular is a very non-critical dependency that is by default "off", so a bug isn't likely to affect most people. Npgsql also has a pretty extensive regression test suite, so any meaningful bugs should get caught before releasing a version.

@roji

This comment has been minimized.

Show comment
Hide comment
@roji

roji Mar 4, 2017

Member

Also since you are mimicking MS SQL Server driver, it may be helpful to change namespace from "Using Npgsql" to "using System.Data.Npgsql".

Although Microsoft's driver is in System.Data, I don't think ADO.NET drivers in general are under System.Data (see MySQL, Oracle...). It seems System.Data is generally reserved for Microsoft packages rather than 3rd-party. Regardless, changing the namespace would be a breaking change with zero added value.

Member

roji commented Mar 4, 2017

Also since you are mimicking MS SQL Server driver, it may be helpful to change namespace from "Using Npgsql" to "using System.Data.Npgsql".

Although Microsoft's driver is in System.Data, I don't think ADO.NET drivers in general are under System.Data (see MySQL, Oracle...). It seems System.Data is generally reserved for Microsoft packages rather than 3rd-party. Regardless, changing the namespace would be a breaking change with zero added value.

@jackjwilliams

This comment has been minimized.

Show comment
Hide comment
@jackjwilliams

jackjwilliams Mar 8, 2017

I started creating an issue when I noticed that my 3.2 update downloaded the entire Microsoft Internet :) (all 865kb of it :>), but I'm glad to see there is already discussion on this.

What was the final decision @roji ?

jackjwilliams commented Mar 8, 2017

I started creating an issue when I noticed that my 3.2 update downloaded the entire Microsoft Internet :) (all 865kb of it :>), but I'm glad to see there is already discussion on this.

What was the final decision @roji ?

@roji

This comment has been minimized.

Show comment
Hide comment
@roji

roji Mar 8, 2017

Member

@jackjwilliams and others, I'll be publishing 3.2.2 very soon, it will still depend on Microsoft.Extensions.Logging. But there's a very good chance the dependency will be removed in 3.2.4.

Member

roji commented Mar 8, 2017

@jackjwilliams and others, I'll be publishing 3.2.2 very soon, it will still depend on Microsoft.Extensions.Logging. But there's a very good chance the dependency will be removed in 3.2.4.

@john2014

This comment has been minimized.

Show comment
Hide comment
@john2014

john2014 Mar 16, 2017

Hi Roji, I got more details directly from PM who work in the .Net Core team about the one file they mentioned and the answer is very surprising.

In .Net Core 2.0, they will merge ALL microsoft nuget (individual) packages into just one huge package called "Microsoft.AspNetCore.All". This will be a huge monolithic package somewhat similar to what we have in .Net Standard Framework.

However the difference is that when you build your app, it will automatically check the dependency and only pull in the required packages. In addition if your webhost server already has those packages, then it wont pull in any packages either. This is done to keep build file size as small as possible.

What this means for Npgsql is that if there is dependency on Microsoft.Logging package, then in .Net Core 2.0, it will pull a huge package "Microsoft.AspNetCore.All" in /bin folder. The file size of this could be around 80-100 MB (which is way more than 865 KB) 😄

.Net 2.0 Beta (Preview 1) will be released around mid may (tentatively). Depending on feedback, the final RTM version will be released 1-2 months later. Hopefully these tips would help in your coding for Npgsql.

john2014 commented Mar 16, 2017

Hi Roji, I got more details directly from PM who work in the .Net Core team about the one file they mentioned and the answer is very surprising.

In .Net Core 2.0, they will merge ALL microsoft nuget (individual) packages into just one huge package called "Microsoft.AspNetCore.All". This will be a huge monolithic package somewhat similar to what we have in .Net Standard Framework.

However the difference is that when you build your app, it will automatically check the dependency and only pull in the required packages. In addition if your webhost server already has those packages, then it wont pull in any packages either. This is done to keep build file size as small as possible.

What this means for Npgsql is that if there is dependency on Microsoft.Logging package, then in .Net Core 2.0, it will pull a huge package "Microsoft.AspNetCore.All" in /bin folder. The file size of this could be around 80-100 MB (which is way more than 865 KB) 😄

.Net 2.0 Beta (Preview 1) will be released around mid may (tentatively). Depending on feedback, the final RTM version will be released 1-2 months later. Hopefully these tips would help in your coding for Npgsql.

@roji

This comment has been minimized.

Show comment
Hide comment
@roji

roji Mar 16, 2017

Member

Thanks for the details @john2014. I think there's one important inaccuracy involved what you say...netstandard20 is indeed going to be a unified library rather than many small nugets. However, that library is going to depend on the actual framework you're targeting. So if you're targeting .NET Core, it makes sense for it to be huge - it contains the implementation for everything. However, when targeting .NET Framework, all this will contain is type forwarders to the actual implementation which is already in you're GAC. This is exactly why the current netstandard libraries weigh less than 1MB. You can try confirming this with your contact.

But regardless, enough people have complained that I'm convinced about dropping Microsoft.Extensions.Logging, which is causing all this. It won't happen immediately, but I hope to be able to do it soon.

Member

roji commented Mar 16, 2017

Thanks for the details @john2014. I think there's one important inaccuracy involved what you say...netstandard20 is indeed going to be a unified library rather than many small nugets. However, that library is going to depend on the actual framework you're targeting. So if you're targeting .NET Core, it makes sense for it to be huge - it contains the implementation for everything. However, when targeting .NET Framework, all this will contain is type forwarders to the actual implementation which is already in you're GAC. This is exactly why the current netstandard libraries weigh less than 1MB. You can try confirming this with your contact.

But regardless, enough people have complained that I'm convinced about dropping Microsoft.Extensions.Logging, which is causing all this. It won't happen immediately, but I hope to be able to do it soon.

@john2014

This comment has been minimized.

Show comment
Hide comment
@john2014

john2014 Mar 16, 2017

However, when targeting .NET Framework, all this will contain is type forwarders to the actual implementation which is already in you're GAC.

If that is the case, then I would strongly recommend sticking with Microsoft.Extension.Logging for the following reasons:

  1. There will be only one additional file (instead of multiple files) installed from Microsoft when installing Npgsql. The main reason why most people are asking about this is the huge number of (duplicate) files it is currently pulling in for .Net Standard version. If it will only pull one small file in .Net Standard framework, most people would be ok with this.
  2. It is always a good idea to use the standard logging platform (if its available, supported and very efficient).
  3. Microsoft has a huge number of very smart people working on it and they will continue to improve the Microsoft.Logging code base (in addition to .Net Core base). Since they are working on both code bases, they can optimize logging much better than LibLog can (unless LibLog want to go through all optimization code and add each optimization code to its library).
  4. Npgsql does not have to rely on 3rd party support (Liblog). It instead can use the base libraries provided by Microsoft. This will also make it future proof. Microsoft can keep updating Microsoft.Logging code base and Npgsql does not have to do anything.

Whenever I install a library, I definitely look for the above factors (security, speed, efficiency, support and future proof - atleast for few months). 😃

john2014 commented Mar 16, 2017

However, when targeting .NET Framework, all this will contain is type forwarders to the actual implementation which is already in you're GAC.

If that is the case, then I would strongly recommend sticking with Microsoft.Extension.Logging for the following reasons:

  1. There will be only one additional file (instead of multiple files) installed from Microsoft when installing Npgsql. The main reason why most people are asking about this is the huge number of (duplicate) files it is currently pulling in for .Net Standard version. If it will only pull one small file in .Net Standard framework, most people would be ok with this.
  2. It is always a good idea to use the standard logging platform (if its available, supported and very efficient).
  3. Microsoft has a huge number of very smart people working on it and they will continue to improve the Microsoft.Logging code base (in addition to .Net Core base). Since they are working on both code bases, they can optimize logging much better than LibLog can (unless LibLog want to go through all optimization code and add each optimization code to its library).
  4. Npgsql does not have to rely on 3rd party support (Liblog). It instead can use the base libraries provided by Microsoft. This will also make it future proof. Microsoft can keep updating Microsoft.Logging code base and Npgsql does not have to do anything.

Whenever I install a library, I definitely look for the above factors (security, speed, efficiency, support and future proof - atleast for few months). 😃

@katie20

This comment has been minimized.

Show comment
Hide comment
@katie20

katie20 Mar 21, 2017

I agree with @john2014 (who opened this bug). If the .Net Core 2.0 will only pull one single file as dependency (with small file size), then it makes sense to stick with Microsoft.Extension.Logging.

Two more reasons would be:

  1. Roji has already implemented Microsoft.Extension.Logging and hence doesnt have to spend time on adding, testing, etc. You can focus on other issues.
  2. LibLog has performance problems and may have other unknown bugs. Roji wont have to focus his time on fixing those issues.

What I would recommend is wait for .Net Core 2.0 Beta to be released (in around 8 weeks). If the file size is small (865KB instead of 80+ MB), then stick with Microsoft.Extension.Logging. If not, you can always go back to LibLog. Meanwhile if users complain about this, you can always point them to this issue discussion.

katie20 commented Mar 21, 2017

I agree with @john2014 (who opened this bug). If the .Net Core 2.0 will only pull one single file as dependency (with small file size), then it makes sense to stick with Microsoft.Extension.Logging.

Two more reasons would be:

  1. Roji has already implemented Microsoft.Extension.Logging and hence doesnt have to spend time on adding, testing, etc. You can focus on other issues.
  2. LibLog has performance problems and may have other unknown bugs. Roji wont have to focus his time on fixing those issues.

What I would recommend is wait for .Net Core 2.0 Beta to be released (in around 8 weeks). If the file size is small (865KB instead of 80+ MB), then stick with Microsoft.Extension.Logging. If not, you can always go back to LibLog. Meanwhile if users complain about this, you can always point them to this issue discussion.

@roji

This comment has been minimized.

Show comment
Hide comment
@roji

roji Mar 21, 2017

Member

@john2014, @katie20, I appreciate your thoughts and the time taken to write them. Although I think the LibLog approach is the right one for library logging, it's true that it simply isn't ready yet. But there's another option which I've chosen for 3.2.2 - simply rollback to the way things were done in Npgsql 3.1, i.e. a custom API which the user can use to inject any logging provider. At the end of the day there simply doesn't seem to be enough value in using Microsoft.Extensions.Logging which justifies the pain it causes via the dependencies.

See #1504 for more details - I hope this solution will be satisfactory for everyone. I'm definitely open to revisiting this later (e.g. when LibLog is more ready).

Member

roji commented Mar 21, 2017

@john2014, @katie20, I appreciate your thoughts and the time taken to write them. Although I think the LibLog approach is the right one for library logging, it's true that it simply isn't ready yet. But there's another option which I've chosen for 3.2.2 - simply rollback to the way things were done in Npgsql 3.1, i.e. a custom API which the user can use to inject any logging provider. At the end of the day there simply doesn't seem to be enough value in using Microsoft.Extensions.Logging which justifies the pain it causes via the dependencies.

See #1504 for more details - I hope this solution will be satisfactory for everyone. I'm definitely open to revisiting this later (e.g. when LibLog is more ready).

@john2014

This comment has been minimized.

Show comment
Hide comment
@john2014

john2014 Mar 21, 2017

Thanks for the update Roji. You may also want to update docs to remove reference to Microsoft.Logging

http://www.npgsql.org/doc/logging.html and http://www.npgsql.org/doc/migration/3.2.html

john2014 commented Mar 21, 2017

Thanks for the update Roji. You may also want to update docs to remove reference to Microsoft.Logging

http://www.npgsql.org/doc/logging.html and http://www.npgsql.org/doc/migration/3.2.html

@roji

This comment has been minimized.

Show comment
Hide comment
@roji

roji Mar 21, 2017

Member

You're right, there's an issue with the docs generation program, hopefully I'll get that working in the next few days....

Member

roji commented Mar 21, 2017

You're right, there's an issue with the docs generation program, hopefully I'll get that working in the next few days....

@roji

This comment has been minimized.

Show comment
Hide comment
@roji

roji Mar 22, 2017

Member

Blocking on dotnet/docfx#1254, will try to skip the API reference generation.

Member

roji commented Mar 22, 2017

Blocking on dotnet/docfx#1254, will try to skip the API reference generation.

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