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

feat(copyright): SPDX copyright detection added #2276

Merged
merged 1 commit into from
Sep 6, 2022

Conversation

rohitpandey49
Copy link
Contributor

@rohitpandey49 rohitpandey49 commented Jul 26, 2022

Signed-off-by: Rohit Pandey rohit.pandey4900@gmail.com

Description

As discussed in issue #1592 , implements a part of the REUSE.Software specification, by detecting the SPDX-FileCopyrightText keyword from .license suffixed files, and applying the copyright detected by the Copyright agent to the associated file.

Changes

  1. New regex added to identify SPDX copyright statement.
  2. Applied copyright detected in .license file to the associated file.

How to test

  1. Start new upload - you may want to use example package available here: https://github.com/fossology/fossology/wiki/ReSo-%28REUSE.Software%29#testing
  2. Select REUSE.Software Analysis and Copyright option.
    Screenshot from 2022-08-09 16-10-15
  3. Start Upload
  4. Check the copyright applied to associated file of .license file.

@shaheemazmalmmd shaheemazmalmmd added GSoC-22 Label to tag issues and pull request for GSOC 2022 activities needs code review needs test labels Jul 28, 2022
@rohitpandey49 rohitpandey49 changed the title feat(copyright): SPDX copyright detection added feat(copyright): SPDX copyright detection added WIP Aug 2, 2022
@GMishx GMishx added the WIP label Aug 8, 2022
@rohitpandey49 rohitpandey49 changed the title feat(copyright): SPDX copyright detection added WIP feat(copyright): SPDX copyright detection added Aug 9, 2022
@GMishx GMishx removed the WIP label Aug 9, 2022
@GMishx GMishx self-requested a review August 9, 2022 12:13
@GMishx
Copy link
Member

GMishx commented Aug 12, 2022

@rohitpandey49 Indeed there are empty space at the end of line in src/reso/agent/ResoAgent.php but on line 199 and 208

image

src/copyright/ui/CopyrightView.php Outdated Show resolved Hide resolved
src/copyright/ui/HistogramBase.php Outdated Show resolved Hide resolved
src/copyright/ui/HistogramBase.php Outdated Show resolved Hide resolved
src/copyright/ui/Xpview.php Outdated Show resolved Hide resolved
src/lib/php/Dao/CopyrightDao.php Outdated Show resolved Hide resolved
src/reso/agent/ResoAgent.php Outdated Show resolved Hide resolved
src/reso/agent/ResoAgent.php Outdated Show resolved Hide resolved
src/spdx2/agent/spdx2.php Outdated Show resolved Hide resolved
src/spdx2/agent/spdx2.php Outdated Show resolved Hide resolved
src/www/ui/ui-export-list.php Outdated Show resolved Hide resolved
@GMishx
Copy link
Member

GMishx commented Aug 16, 2022

Additional note @rohitpandey49 , please check if the copyright table does not already have entry for the given set of agent_fk, pfile_fk and hash otherwise it will cause duplicate entries for copyright if same file is scanned multiple times.

@rohitpandey49
Copy link
Contributor Author

@GMishx Sir please check.

@github-actions github-actions bot added the has merge conflicts PR to be rebased label Aug 24, 2022
@github-actions
Copy link

This pull request has conflicts, please rebase with master to resolve those before we can evaluate the pull request.

@shaheemazmalmmd
Copy link
Contributor

@rohitpandey49 : please rebase the branch with master and resolve the conflicts.

@github-actions github-actions bot removed the has merge conflicts PR to be rebased label Aug 28, 2022
@rohitpandey49 rohitpandey49 force-pushed the gsoc22/spdx-copyright branch 2 times, most recently from ef617a0 to f081ce7 Compare August 28, 2022 04:12
@GMishx
Copy link
Member

GMishx commented Sep 2, 2022

@rohitpandey49 , changes looks good to me. Please add following changes to the code as well.

  1. The change in CopyrightDao.php is a boolean logic fix (invert AND with OR and vice-versa if the condition is changed). This also fixes the test case.
  2. The change in ResoAgentPlugin.php allows reso to add copyright agent as a dependency if user has selected it.
diff --git a/src/lib/php/Dao/CopyrightDao.php b/src/lib/php/Dao/CopyrightDao.php
index 95390cf2f..c833de25f 100644
--- a/src/lib/php/Dao/CopyrightDao.php
+++ b/src/lib/php/Dao/CopyrightDao.php
@@ -61,7 +61,7 @@ class CopyrightDao
     $statementName = __METHOD__.$tableName;
     $params = array($pFileId);
     $addAgentValue = "";
-    if (!empty($agentId) || $agentId[0] != 0) {
+    if (!empty($agentId) && $agentId[0] != 0) {
       $agentIds = implode(",", $agentId);
       $statementName .= '.agentId';
       $addAgentValue = ' AND agent_fk= ANY($2::int[])';
diff --git a/src/reso/ui/ResoAgentPlugin.php b/src/reso/ui/ResoAgentPlugin.php
index 1b44d6b21..31a12fa04 100644
--- a/src/reso/ui/ResoAgentPlugin.php
+++ b/src/reso/ui/ResoAgentPlugin.php
@@ -38,7 +38,12 @@ class ResoAgentPlugin extends AgentPlugin
    */
   public function AgentAdd($jobId, $uploadId, &$errorMsg, $dependencies=array(), $arguments=null)
   {
+    $copyrightAgentScheduled = GetParm("Check_agent_copyright",
+        PARM_INTEGER) == 1;
     $dependencies[] = "ojo";
+    if ($copyrightAgentScheduled) {
+      $dependencies[] = "copyright";
+    }
     if ($this->AgentHasResults($uploadId) == 1) {
       return 0;
     }

@rohitpandey49
Copy link
Contributor Author

@GMishx Sir, updated. Please check.

@GMishx
Copy link
Member

GMishx commented Sep 5, 2022

Sorry @rohitpandey49 , I messed the dependency check. Can you please modify it like following? We can merge the PR after that.

diff --git a/src/reso/ui/ResoAgentPlugin.php b/src/reso/ui/ResoAgentPlugin.php
index 279d6202b..714a3b809 100644
--- a/src/reso/ui/ResoAgentPlugin.php
+++ b/src/reso/ui/ResoAgentPlugin.php
@@ -39,9 +39,9 @@ class ResoAgentPlugin extends AgentPlugin
   public function AgentAdd($jobId, $uploadId, &$errorMsg, $dependencies=array(), $arguments=null)
   {
     $copyrightAgentScheduled = GetParm("Check_agent_copyright", PARM_INTEGER) == 1;
-    $dependencies[] = "ojo";
+    $dependencies[] = "agent_ojo";
     if ($copyrightAgentScheduled) {
-      $dependencies[] = "copyright";
+      $dependencies[] = "agent_copyright";
     }
     if ($this->AgentHasResults($uploadId) == 1) {
       return 0;
@@ -52,7 +52,7 @@ class ResoAgentPlugin extends AgentPlugin
       return $jobQueueId;
     }
 
-    return $this->doAgentAdd($jobId, $uploadId, $errorMsg, array("agent_ojo"),$uploadId);
+    return $this->doAgentAdd($jobId, $uploadId, $errorMsg, $dependencies, $uploadId);
   }
 
   /**

Signed-off-by: rohitpandey49 <rohit.pandey4900@gmail.com>
@rohitpandey49
Copy link
Contributor Author

@GMishx Sir updated. Please check.

Copy link
Member

@GMishx GMishx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes looks good. Tested, working as expected.

@GMishx GMishx merged commit 44d179f into fossology:master Sep 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GSoC-22 Label to tag issues and pull request for GSOC 2022 activities ready
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants