pivotal / LicenseFinder

Find licenses for your project's dependencies.
MIT License
1.71k stars 338 forks source link

Adding some license detection via url for Nuget #900

Open RemakingEden opened 2 years ago

RemakingEden commented 2 years ago

Hi,

First off thanks for all the hard work, I use this on an NPM project and it works great.

I am trying to use this on a dotnet project now and running into the fact that license detection is not currently working. I am hoping this could be a fix I could do as it seems that many of the URL's for the licenses are pretty standard e.g. https://licenses.nuget.org/MIT||Apache-2.0

image

I have looked through the code and I have no experience with Ruby so I was hoping someone could give me some pointers. I hoped simply adding a new match for the matcher in definitions may simply work but this doesnt seem to be the case. Does the actual matching need to be enabled somewhere for nuget?

image

Thanks for any advice!

xtreme-shane-lattanzio commented 2 years ago

Hi @RemakingEden ! Sorry for the delay. I looked into this and I don't think its due to the matching. It was pulling from the wrong spot to look for a license so it wasn't even trying to match. I made a PR here: https://github.com/pivotal/LicenseFinder/pull/903 to see if the tests pass. I may have to tweak it a bit more but i did manage to fix the unknowns in a test project with that one line fix

RemakingEden commented 2 years ago

Thanks @xtreme-shane-lattanzio! I've just pulled your branch and it seems to run well. It still unfortunately won't match the MIT license with the URL https://licenses.nuget.org/MIT with the matching code I put above. To elaborate im trying to make it so the response that currently looks like

xunit.runner.visualstudio, 2.4.3, https://licenses.nuget.org/MIT

will look like this

xunit.runner.visualstudio, 2.4.3, MIT
RemakingEden commented 2 years ago

This is the matching code I am trying to use

       def mit
         url_regexp = %r{MIT Licen[sc]e.*http://(?:www\.)?opensource\.org/licenses/mit-license}
+        alternate_regexp = 'https://licenses.nuget.org/MIT'
         header_regexp = /The MIT Licen[sc]e/
         one_liner_regexp = /is released under the MIT licen[sc]e/

         matcher = AnyMatcher.new(
           Matcher.from_template(Template.named('MIT')),
           Matcher.from_regex(url_regexp),
+          Matcher.from_text(alternate_regexp),
           HeaderMatcher.new(Matcher.from_regex(header_regexp)),
           Matcher.from_regex(one_liner_regexp)
         )
xtreme-shane-lattanzio commented 2 years ago

@RemakingEden Oh I think I misunderstood. I thought that that screenshot was desired output. When I ran this I seemed to just get unknown for all licenses. This was because it was only looking in the current repo for the packages to populate the information. If that is sufficient (I dont know much about nuget) then my change isn't any good.

There are many layers to the matching. First it will check if the Package/NugetPackage object was populated with license information. If it was, it checks based on the name and tries to compare to the definitions in the file you posted. If it does not, it will just forward the same name which is what is happening here.

If there is no license information populated, the next step is to look for license files and then compare the text to see if it matches. This is the part you changed but this code is never reached because there is license information in the package. Even so, I think that URL regex is there in case the URL is actually inside the text which does not seem to be the case here.

So all this means it will try to match on a name which in this case seems to be https://licenses.nuget.org/MIT, it will find nothing and just print it out as is. It does not seem right to me to add this as an alternative name for the license since the license.rb file has no concept of package manager. I think the right fix here is to strip the URL part from the name in the nuget specific code. As per your example, this should work most of the time and then the matching will start to work based on name .

I tried to do that. Can you let me know if it fixes things for you?

RemakingEden commented 2 years ago

@xtreme-shane-lattanzio Sorry thats probably me not explaining well enough.

Thanks for explaining the matching that makes a little more sense now. I took your branch and built the container and I am still not getting any matching unfortunately. image

This is the project I am working on if it helps debug https://github.com/Project-MONAI/monai-deploy-workflow-manager

I think the stripping makes sense as most of the licenses are laid out in the same way and it seems Nuget is trying to move towards this standardisation. For me it would just be great if I could whitelist MIT, Apache 2.0 and cover 60% of the packages without manually adding each one. Let me know if you need any more info.

xtreme-shane-lattanzio commented 2 years ago

@RemakingEden Am I missing something? I do not see any package manager file in that repo so there are no dependencies detected. I'm looking for a packages.config or sln file or maybe a .nuget file to trigger the nuget package manager. Or am I supposed to be running something that will pull in nupkgs?

RemakingEden commented 2 years ago

@xtreme-shane-lattanzio Sorry we are working on the develop branch. So if youre using it in the container you will need to do these below steps.

  1. Install Dotnet 6
  sudo apt-get update \
  sudo apt-get install -y apt-transport-https && \
  sudo apt-get update && \
  sudo apt-get install -y dotnet-sdk-6.0
  1. Clone repo and checkout to develop
  2. Restore packages from sln file dotnet restore src/Monai.Deploy.WorkflowManager.sln
  3. The run license finder recursively license_finder -r

Let me know if that still doesnt work

xtreme-shane-lattanzio commented 2 years ago

@RemakingEden Ah I see the issue, I needed to make the change in the dotnet package manager and not nuget. I pushed a change for it now!

RemakingEden commented 2 years ago

@RemakingEden Ah I see the issue, I needed to make the change in the dotnet package manager and not nuget. I pushed a change for it now!

That works brilliantly. Exactly as expected.

Thanks so much for your help!