Skip to content

feat(copyright): allow copyright to run standalone #1155

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
Sep 14, 2018
Merged

feat(copyright): allow copyright to run standalone #1155

merged 1 commit into from
Sep 14, 2018

Conversation

maxhbr
Copy link
Member

@maxhbr maxhbr commented Aug 7, 2018

For the command line usage of copyright, no connection to the DB is necessary.

Previously the DbConnection was initializen on every start of copyright, with the changes here the initialization is moved to the if-branch, which corresponds to the scheduler usage. This allows to use the copyright agent stand-alone without the need of a DB or a configuration.

@maxhbr maxhbr added the WIP label Aug 7, 2018
@ghost ghost assigned maxhbr Aug 7, 2018
@ghost ghost added needs code review and removed WIP labels Aug 7, 2018
@GMishx
Copy link
Member

GMishx commented Aug 9, 2018

Thanks @maxhbr , this was a much required feature. It is working fine as a CLI agent but can not communicate with FOSSology as an agent.

2018-08-09 10_27_55-ubuntu running - oracle vm virtualbox _ 2

@GMishx GMishx added the WIP label Aug 9, 2018
@maxhbr
Copy link
Member Author

maxhbr commented Aug 9, 2018

The stupid wafflebot has removed the WIP label without a reason...
2018-08-09_07 47 15
I should have seen that and added it back in.

@maxhbr
Copy link
Member Author

maxhbr commented Aug 9, 2018

The problem is. that there are conflictig rules for WIP and "new PRs":
2018-08-09_08 10 50
one can not create a new PR with the WIP label...

@maxhbr maxhbr force-pushed the maxhbr/makeCopyrightCliIdepententFromDB branch from 5c6068d to efe6744 Compare August 13, 2018 06:46
@ghost ghost added needs code review and removed WIP labels Aug 13, 2018
@maxhbr maxhbr force-pushed the maxhbr/makeCopyrightCliIdepententFromDB branch from efe6744 to aa0343d Compare August 13, 2018 06:57
@ghost ghost added needs code review and removed WIP labels Aug 13, 2018
@maxhbr maxhbr force-pushed the maxhbr/makeCopyrightCliIdepententFromDB branch from aa0343d to d20f385 Compare August 13, 2018 07:53
@ghost ghost added needs code review and removed WIP labels Aug 13, 2018
@GMishx
Copy link
Member

GMishx commented Aug 13, 2018

One quick suggestion here, rename the PR with WIP: xxx instead of fighting with the bot 😉

@maxhbr maxhbr removed their assignment Aug 13, 2018
@GMishx
Copy link
Member

GMishx commented Aug 29, 2018

Is the PR ready @maxhbr ?

@maxhbr
Copy link
Member Author

maxhbr commented Aug 29, 2018

Yes, it is labeled as needs code review and needs test without WIP.

@GMishx GMishx self-assigned this Aug 29, 2018
@GMishx GMishx self-requested a review August 29, 2018 07:56
@jeffmcaffer jeffmcaffer mentioned this pull request Sep 1, 2018
Closed
@GMishx
Copy link
Member

GMishx commented Sep 3, 2018

@maxhbr Please rebase the branch due to #1192 .

@maxhbr
Copy link
Member Author

maxhbr commented Sep 3, 2018

You can always check out the state after the merge via:

$ git fetch "origin" "+refs/pull/1155/merge:"
$ git checkout -b testpr/origin/1155 -qf FETCH_HEAD

and one should always do that, to actually test the correct state.

@maxhbr maxhbr force-pushed the maxhbr/makeCopyrightCliIdepententFromDB branch from d20f385 to 8349600 Compare September 3, 2018 06:45
@ghost ghost assigned maxhbr Sep 3, 2018
@maxhbr
Copy link
Member Author

maxhbr commented Sep 3, 2018 •
edited
Loading

Branch is rebased

Edit: travis fails, since it pulls the branch from the wrong remote...

@maxhbr maxhbr removed their assignment Sep 3, 2018
@GMishx
Copy link
Member

GMishx commented Sep 3, 2018

The code is working with scheduler but it gave me a core dump for ./copyright -h.

Reading symbols from copyright...done.
[New LWP 8279]
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
Core was generated by `./copyright -h'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x000000000043aaae in fo_scheduler_disconnect (retcode=retcode@entry=1) at libfossscheduler.c:355
355	  if (strcmp(sysconfigdir, DEFAULT_SETUP))
(gdb) bt
#0  0x000000000043aaae in fo_scheduler_disconnect (retcode=retcode@entry=1) at libfossscheduler.c:355
#1  0x0000000000411a4e in main (argc=2, argv=0x7ffd9b05bd98) at copyright.cc:47

@maxhbr
Copy link
Member Author

maxhbr commented Sep 3, 2018

I will check that.

@maxhbr maxhbr changed the title feat(copyright): allow copyright to run standalone WIP: feat(copyright): allow copyright to run standalone Sep 3, 2018
@maxhbr maxhbr force-pushed the maxhbr/makeCopyrightCliIdepententFromDB branch from 8349600 to cbecf05 Compare September 3, 2018 09:04
@ghost ghost assigned maxhbr Sep 3, 2018
@maxhbr
feat(copyright): allow copyright to run standalone
f5c89fa 
For the command line usage of copyright, no connection to the DB is necessary.

Previously the DbConnection was initializen on every start of copyright, with
this changes here the initialization is moved to the if branch, which
corresponds to the scheduler usage.
@maxhbr maxhbr force-pushed the maxhbr/makeCopyrightCliIdepententFromDB branch from cbecf05 to f5c89fa Compare September 3, 2018 09:16
@maxhbr
Copy link
Member Author

maxhbr commented Sep 3, 2018

The segfault should be fixed by https://github.com/fossology/fossology/pull/1155/files#diff-349cf04da2e1ce5fd6730721eb3ec205R342

Additionally I have added https://github.com/fossology/fossology/pull/1155/files#diff-d9b27972f680d5d2bb8cc20f4aae4c38R125 to have exit code 0 with the $0 --help call.

@maxhbr maxhbr changed the title WIP: feat(copyright): allow copyright to run standalone feat(copyright): allow copyright to run standalone Sep 3, 2018
@GMishx GMishx mentioned this pull request Sep 3, 2018
Closed
GMishx
GMishx approved these changes Sep 5, 2018
View reviewed changes
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.

Tested, working fine.

@GMishx GMishx removed the needs test label Sep 5, 2018
GMishx
GMishx approved these changes Sep 7, 2018
View reviewed changes
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.

Code looks good.

@maxhbr
Copy link
Member Author

maxhbr commented Sep 11, 2018

@GMishx merge?

@GMishx GMishx assigned ag4ums and unassigned maxhbr and GMishx Sep 13, 2018
@GMishx GMishx merged commit 55eb9a5 into fossology:master Sep 14, 2018
@GMishx GMishx mentioned this pull request Sep 17, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@GMishx GMishx GMishx 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.

3 participants

@maxhbr @GMishx @ag4ums