Skip to content

fix(licenseRef): Fix import of licenseRef.json #1663

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
Merged
merged 1 commit into from
Jul 16, 2020
Merged

fix(licenseRef): Fix import of licenseRef.json #1663

merged 1 commit into from
Jul 16, 2020

Conversation

GMishx
Copy link
Member

@GMishx GMishx commented Mar 13, 2020

Changes

  1. Remove rf_md5 from licenseRef.json as it can be derived from rf_text.
  2. Remove rf_pk from licenseRef.json as rf_shortname is the correct way to reference a license from DB.
  3. Update rf_md5 whenever rf_text updates.
  4. Insert rf_add_date for new licenses.
  5. If a license already exists as candidate, merge it with main license_ref before updating.

How to test

  • On an existing DB:
    1. Install the branch and check if the license texts are updated
    2. The rf_md5 is correct (SELECT rf_shortname, rf_md5, md5(rf_text) AS hash FROM license_ref WHERE rf_md5 != md5(rf_text);)
    3. Check if license_ref.rf_pk has not been changed.
  • On new DB:
    1. Check if all the licenses are updated.
    2. The rf_md5 is correct (SELECT rf_shortname, rf_md5, md5(rf_text) AS hash FROM license_ref WHERE rf_md5 != md5(rf_text);)
  • On existing DB with candidates:
    1. Insert a candidate license.
    2. Insert the same license to licenseRef.json.
    3. After installing, check if candidate license got merged with the license_ref and the rf_pk for the license has not changed.

@GMishx GMishx force-pushed the fix/licenseref/import-fixes branch 4 times, most recently from a622a13 to c89a200 Compare March 19, 2020 08:28
@GMishx GMishx mentioned this pull request Mar 30, 2020
Closed
@ag4ums ag4ums self-assigned this Apr 3, 2020
ag4ums
ag4ums approved these changes Apr 7, 2020
View reviewed changes
Copy link
Contributor

@ag4ums ag4ums left a comment

Choose a reason for hiding this comment

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

code looks good

@ag4ums
Copy link
Contributor

ag4ums commented Jun 3, 2020

updating fo-postinstall is breaking

Update reference licenses
PHP Warning:  pg_execute(): Query failed: ERROR:  bind message supplies 4 parameters, but prepared statement "initLicenseRefTable.updateLicenseRef" requires 3 in /usr/local/share/fossology/lib/php/Db/Driver/Postgres.php on line 70
PHP Stack trace:
PHP   1. {main}() /usr/local/lib/fossology/fossinit.php:0
PHP   2. initLicenseRefTable() /usr/local/lib/fossology/fossinit.php:226
PHP   3. Fossology\Lib\Db\ModernDbManager->getSingleRow() /usr/local/lib/fossology/fossinit.php:575
PHP   4. Fossology\Lib\Db\ModernDbManager->execute() /usr/local/share/fossology/lib/php/Db/DbManager.php:163
PHP   5. Fossology\Lib\Db\Driver\Postgres->execute() /usr/local/share/fossology/lib/php/Db/ModernDbManager.php:65
PHP   6. pg_execute() /usr/local/share/fossology/lib/php/Db/Driver/Postgres.php:70
[2020-06-03 13:48:25] /usr/local/share/fossology/lib/php/libschema.php.CRITICAL: ERROR:  bind message supplies 4 parameters, but prepared statement "initLicenseRefTable.updateLicenseRef" requires 3 [] []
PHP Fatal error:  Uncaught Fossology\Lib\Exception: error executing: initLicenseRefTable.updateLicenseRef: UPDATE license_ref set rf_text=$1,rf_md5=md5($2) WHERE rf_shortname=$3; -- -- Array
(
    [0] => this license is added by.
    [1] => this license is added by.
    [2] => https://test.test.com/licenses/universal-foss-exception/
    [3] => test
)

 

ERROR:  bind message supplies 4 parameters, but prepared statement "initLicenseRefTable.updateLicenseRef" requires 3 in /usr/local/share/fossology/lib/php/Db/DbManager.php:144
Stack trace:
#0 /usr/local/share/fossology/lib/php/Db/ModernDbManager.php(68): Fossology\Lib\Db\DbManager->checkResult(false, 'initLicenseRefT...')
#1 /usr/local/share/fossology/lib/php/Db/DbManager.php(163): Fossology\Lib\Db\ModernDbManager->execute('initLicenseRefT...', Array)
#2 /usr/local/lib/fossology/fossinit.php(575): Fossology\Lib\Db\DbManager->getSingleRow('UPDATE license_...', Array, 'initLicenseRefT...')
#3 /usr/local/lib/fossology/fossinit.php(226): initLicenseRefTable(false)
#4 { in /usr/local/share/fossology/lib/php/Db/DbManager.php on line 144

@GMishx GMishx force-pushed the fix/licenseref/import-fixes branch from c89a200 to e8c7c13 Compare June 11, 2020 10:15
@GMishx
fix(licenseRef): Fix import of licenseRef.json
865f8ac 
1. Remove rf_md5 from licenseRef.json as it can be derived from rf_text.
2. Remove rf_pk from licenseRef.json as rf_shortname is the correct way
   to reference a license from DB.
3. Update rf_md5 whenever rf_text updates.
4. Insert rf_add_date for new licenses.
5. If a license already exists as candidate, merge it with main
   license_ref before updating.

Signed-off-by: Gaurav Mishra <[email protected]>
@GMishx GMishx force-pushed the fix/licenseref/import-fixes branch from e8c7c13 to 865f8ac Compare July 9, 2020 05:33
@GMishx
Copy link
Member Author

GMishx commented Jul 9, 2020

updating fo-postinstall is breaking

Hello @ag4ums , I've fixed the issue. Can you please test again?

@GMishx GMishx removed the WIP label Jul 9, 2020
@GMishx GMishx requested a review from ag4ums July 9, 2020 05:35
ag4ums
ag4ums approved these changes Jul 16, 2020
View reviewed changes
Copy link
Contributor

@ag4ums ag4ums left a comment

Choose a reason for hiding this comment

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

tested working fine

@ag4ums ag4ums merged commit 6502333 into master Jul 16, 2020
@ag4ums ag4ums deleted the fix/licenseref/import-fixes branch July 16, 2020 07:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ag4ums ag4ums ag4ums approved these changes

Assignees

@ag4ums ag4ums

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@GMishx @ag4ums