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

Major refactoring of the folders #9

Closed
wants to merge 1 commit into from
Closed

Conversation

massix
Copy link

@massix massix commented May 1, 2012

Since we've decided to keep the two projects separated it's necessary
that if some one wants to clone the repository he has a clear idea of
what's going on here: in src/lib folder there are the three main
components of the library libgrive (drive, protocol and util); while
in the src/cli folder there's the command line interface tool that uses
the library. The CLI is called in the same way as the library (grive).

In this way, we can keep multiple clients using the same library in just
one repo..

These are the output of compiling the new tree:
pearl :: Developer/grive/build » cmake .. -- The C compiler identification is GNU -- The CXX compiler identification is GNU -- Checking whether C compiler has -isysroot -- Checking whether C compiler has -isysroot - yes -- Checking whether C compiler supports OSX deployment target flag -- Checking whether C compiler supports OSX deployment target flag - yes -- Check for working C compiler: /usr/bin/gcc -- Check for working C compiler: /usr/bin/gcc -- works -- Detecting C compiler ABI info -- Detecting C compiler ABI info - done -- Checking whether CXX compiler has -isysroot -- Checking whether CXX compiler has -isysroot - yes -- Checking whether CXX compiler supports OSX deployment target flag -- Checking whether CXX compiler supports OSX deployment target flag - yes -- Check for working CXX compiler: /usr/bin/c++ -- Check for working CXX compiler: /usr/bin/c++ -- works -- Detecting CXX compiler ABI info -- Detecting CXX compiler ABI info - done -- checking for module 'openssl' -- found openssl, version 0.9.8j -- Found OpenSSL: /usr/lib/libssl.dylib;/usr/lib/libcrypto.dylib (found version "0.9.8") -- Found JSON-C: /usr/local/lib/libjson.dylib -- Found CURL: /usr/lib/libcurl.dylib -- Found CppUnit: /usr/local/lib/libcppunit.dylib -- Building unitary tests along with the library and the binary -- Configuring done -- Generating done -- Build files have been written to: /Users/massi/Developer/grive/build

Compiling...
pearl :: Developer/grive/build » make Scanning dependencies of target grive [ 6%] Building CXX object src/lib/CMakeFiles/grive.dir/drive/Collection.cc.o [ 13%] Building CXX object src/lib/CMakeFiles/grive.dir/drive/Drive.cc.o [ 20%] Building CXX object src/lib/CMakeFiles/grive.dir/protocol/Download.cc.o [ 26%] Building CXX object src/lib/CMakeFiles/grive.dir/protocol/HTTP.cc.o [ 33%] Building CXX object src/lib/CMakeFiles/grive.dir/protocol/Json.cc.o [ 40%] Building CXX object src/lib/CMakeFiles/grive.dir/protocol/OAuth2.cc.o [ 46%] Building CXX object src/lib/CMakeFiles/grive.dir/util/Crypt.cc.o [ 53%] Building CXX object src/lib/CMakeFiles/grive.dir/util/DateTime.cc.o [ 60%] Building CXX object src/lib/CMakeFiles/grive.dir/util/OS.cc.o [ 66%] Building CXX object src/lib/CMakeFiles/grive.dir/util/Path.cc.o Linking CXX shared library libgrive.dylib [ 66%] Built target grive Scanning dependencies of target grive_executable [ 73%] Building CXX object src/cli/CMakeFiles/grive_executable.dir/main.cc.o Linking CXX executable grive [ 73%] Built target grive_executable Scanning dependencies of target unittest [ 80%] Building CXX object test/CMakeFiles/unittest.dir/UnitTest.cc.o [ 86%] Building CXX object test/CMakeFiles/unittest.dir/util/DateTimeTest.cc.o [ 93%] Building CXX object test/CMakeFiles/unittest.dir/util/FunctionTest.cc.o [100%] Building CXX object test/CMakeFiles/unittest.dir/util/PathTest.cc.o Linking CXX executable unittest [100%] Built target unittest

The output of ldd
pearl :: Developer/grive/build » otool -L ./src/cli/grive ./src/cli/grive: /Users/massi/Developer/grive/build/src/lib/libgrive.0.dylib (compatibility version 0.0.0, current version 0.0.1) /usr/lib/libcurl.4.dylib (compatibility version 7.0.0, current version 7.0.0) /usr/local/lib/libjson.0.dylib (compatibility version 1.0.0, current version 1.1.0) /usr/lib/libssl.0.9.8.dylib (compatibility version 0.9.8, current version 44.0.0) /usr/lib/libcrypto.0.9.8.dylib (compatibility version 0.9.8, current version 44.0.0) /usr/lib/libstdc++.6.dylib (compatibility version 7.0.0, current version 52.0.0) /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 159.1.0)

Running the unittest..
`pearl :: Developer/grive/build » ./test/unittest
........

OK (8 tests)`

Running the grive CLI..

`pearl :: Developer/grive/build 130 » ./src/cli/grive -a

Please go to this URL and get an authenication code:

https://accounts.google.com/o/oauth2/auth?scope=https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fuserinfo.email+https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fuserinfo.profile+https%3A%2F%2Fdocs.google.com%2Ffeeds%2F+https%3A%2F%2Fdocs.googleusercontent.com%2F+https%3A%2F%2Fspreadsheets.google.com%2Ffeeds%2F&redirect_uri=urn:ietf:wg:oauth:2.0:oob&response_type=code&client_id=22314510474.apps.googleusercontent.com


Please input the authenication code here
^C`

Installing...
pearl :: Developer/grive/build 130 » make install [ 66%] Built target grive [ 73%] Built target grive_executable [100%] Built target unittest Install the project... -- Install configuration: "" -- Installing: /usr/local/lib/libgrive.0.0.1.dylib -- Up-to-date: /usr/local/lib/libgrive.0.dylib -- Up-to-date: /usr/local/lib/libgrive.dylib -- Up-to-date: /usr/local/include/grive/drive/Collection.hh -- Up-to-date: /usr/local/include/grive/drive/Drive.hh -- Up-to-date: /usr/local/include/grive/protocol/Download.hh -- Up-to-date: /usr/local/include/grive/protocol/HTTP.hh -- Up-to-date: /usr/local/include/grive/protocol/Json.hh -- Up-to-date: /usr/local/include/grive/protocol/OAuth2.hh -- Up-to-date: /usr/local/include/grive/util/Crypt.hh -- Up-to-date: /usr/local/include/grive/util/DateTime.hh -- Up-to-date: /usr/local/include/grive/util/Function.hh -- Up-to-date: /usr/local/include/grive/util/OS.hh -- Up-to-date: /usr/local/include/grive/util/Path.hh -- Installing: /usr/local/bin/grive

Since we've decided to keep the two projects separated it's necessary
that if some one wants to clone the repository he has a clear idea of
what's going on here: in src/lib folder there are the three main
components of the library libgrive (drive, protocol and util); while
in the src/cli folder there's the command line interface tool that uses
the library. The CLI is called in the same way as the library (grive).

In this way, we can keep multiple clients using the same library in just
one repo..
@match065 match065 closed this May 2, 2012
@match065
Copy link
Member

match065 commented May 2, 2012

I am merging fix-interrupt.

Meanwhile, I am changing lib to libgrive, cli to grive. OK?

BTW, I have a few things to say about the SignalHandler class.

  1. What is the use of SignalFunctor?
  2. I saw you are using raw function pointer in SignalHandler. Any chance you can use Function<void(int)> instead?
  3. No need to implement copy contructor and operator= (I think you mean operator= instead of ==, typo?) If someone accidentally calls them just let them have link error.
  4. SignalHandler::UnregisterSignal(): setting the pointer to 0 doesn't remove it from map. Is it intended?
  5. SignalHandler::RegisterSignal(): why not use map::find()?
  6. BTW, why do we need to keep track of signals like this?

@massix
Copy link
Author

massix commented May 2, 2012

Meanwhile, I am changing lib to libgrive, cli to grive. OK?

Ok, no problems for me :)

I am merging fix-interrupt.

It's not well tested yet, if you want to still merge it please consider that I didn't open the pull request yet.. I was just showing you the code to know if it's compliant with the coding rules you've defined in the wiki :P

What is the use of SignalFunctor?
I saw you are using raw function pointer in SignalHandler. Any chance you can use Function instead?

Basically, I wanted the SignalFunctor to be a wrapper around a Function object.. still it's not the case :-P

No need to implement copy contructor and operator= (I think you mean operator= instead of ==, typo?) If someone accidentally calls them just let them have link error.

It's a matter of taste.. If I want the APIs to be clearer I usually define them as private (but even not defining them at all might be a good choice).. regarding operator== yes, it's a typo :(

SignalHandler::UnregisterSignal(): setting the pointer to 0 doesn't remove it from map. Is it intended?
SignalHandler::RegisterSignal(): why not use map::find()?

That's because at first I was using a vector and dealing with the object in a different way.. since I've now converted it to a map yes, it's better to use map::find() and redesign the algorithm in a proper way (still, I've told you I wasn't ready for the pull yet :P)

BTW, why do we need to keep track of signals like this?

Basically, because I want to make sure that you can't override a signal two times straight, in this way we can always get back to the default handler of the signal (which is the OS defined one).. by using this solution we can enable a specific handler for a specific signal only in some parts of the code (like when we're downloading a file) and get back to the original handler in an easy way (just calling the UnregisterSignal method).

Now I'm at work and probably I won't have time to deal with grive until this weekend.. feel free to enhance the SignalHandler if you pull it.. I will just re-fork everything and re-start coding from where you left this weekend :-)

@match065
Copy link
Member

match065 commented May 2, 2012

oops... I pull that and releasing v0.0.3 now. I guess I should comment out SignalHandler before v0.0.3. Sorry for pulling that without your nod.

I still don't quite get the benefit of changing the signal handler in some parts of the code. Are you afraid of grive being ctrl-C'ed and leaving things inconsistent? I don't worry that much because the files are protected by checksums.

Anyway, I will leave the SignalHandler to your good hands.

For the copy ctor & operator=, it is a famous idiom to make them private to prevent people from calling them (and prevent the compiler from generating them too). Normally people will just declare them in the header without actually implementing them in the .cc file.

@match065
Copy link
Member

match065 commented May 2, 2012

And thanks! See you in the weekend!

@indian4646 indian4646 mentioned this pull request Jun 15, 2012
@jorgebr jorgebr mentioned this pull request Jun 25, 2012
@eyerouge eyerouge mentioned this pull request Jul 16, 2012
@ramsiw ramsiw mentioned this pull request May 16, 2013
@elbaulp elbaulp mentioned this pull request Oct 21, 2013
@farleylai farleylai mentioned this pull request Dec 12, 2013
@splineGear splineGear mentioned this pull request May 14, 2014
@ghost ghost mentioned this pull request Dec 2, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants