The bazel dependency is maybe excessive

JeffAbrahamson commented 8 years ago

I have an ongoing conversation with the bazel folks about getting bazel to work well enough that I can use cctz.

Since that conversation still ongoing, I wrote a quick makefile that makes libcctz.a and runs its tests. To be truly useful, it should probably be a cmakefile, but I'm happy to contribute my simple makefile in case that can help.

I understand if you don't want to go in that direction, and what I've done isn't overly complicated for others to redo as needed. It was faster than bringing up bazel just to compile cctz, though.

JeffAbrahamson commented 8 years ago

Fwiw, this repo is what I did.

devjgm commented 8 years ago

JFYI, I'm not ignoring this, I've just been sick. I'll be back soon. Thanks.

devjgm commented 8 years ago

We use bazel to build and test cctz, but those who want to use it are not required to use bazel. I'd imagine that, for example, for a Mac OS X app a developer would add the cctz source files to their Xcode project and compile it right in.

So in general, I think we want to stick with bazel for the development and testing of cctz itself.

However, I am somewhat interested in including a small and simple Makefile with the cctz project. It would only compile the examples and the time_tool. It would not fetch googletest, build, or run any of the unit tests. This simple Makefile could serve as an outline for how developers can compile cctz in their own projects.

JeffAbrahamson commented 8 years ago

Sure, that's quite reasonable and what I'd expect.

So look at the gmake directory in my cctz repo. It's the only thing I changed. It's mildly more complex than just a plain make because bazel has its google3 heritage of full paths everywhere and so it's convenient outside that context to strip the prepending 'src/' strings, which it handles on the fly with a couple sed's. (Maybe there was another way, but this is what I saw first and was easy.)

davidzchen commented 8 years ago

Would it be simpler to keep the full include paths using non-recursive make?

JeffAbrahamson commented 8 years ago

Maybe. Easy is a function of whose brain at what moment in time with what set of distractions at the moment.

SemenMartynov commented 8 years ago

Jeff, can you add -lpthread into your Makefile for the tool? Otherwise, it calls this error https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55394

JeffAbrahamson commented 8 years ago

Good call. I've done this. The code is here.

And this is where my git-fu and my github-fu cross in the night. I foolishly didn't make the changes on a branch, so it's harder than it should be for me to send you a patch. And I didn't fork the project using github, but, rather, just added a new remote on my end. I think. So github isn't facilitating creating a pull request.

Git lets me create a patch against a different remote, so that's easy enough. (My apologies in advance for not seeing a better way.)

jeff@birdsong:cctz $ git remote
jellybooks
upstream
jeff@birdsong:cctz $ git format-patch upstream/master
0001-Add-makefiles.patch
0002-Add-install-target.patch
0003-Install-header-file-on-install.patch
0004-Note-use-of-pthreads-at-compile-time.patch
jeff@birdsong:cctz $

Github, on the other hand, doesn't permit uploading patch files. Let me know what's easiest for your workflow.

JeffAbrahamson commented 8 years ago

Checking in on the makefile patch:

Are you waiting for something from me? If so, I misunderstood, could you tell me again?
Is there a better form in which I should submit this?
Is this just normal free-software-but-there's-life-first delay?
Did this dead-end for some reason?

Thanks.

devjgm commented 8 years ago

Hi, Jeff. We're very sorry about the delay. No excuse, but the reason is that we've been very busy preparing two papers for the C++ standard meeting next week. We're presenting proposals for standard C++ Civil-Time Library and a standard Time-Zone Library (https://github.com/devjgm/papers).

Along with these papers we're preparing a somewhat large update to cctz that will match the design presented in those papers. The fundamental concepts are the same as those that are in cctz currently. But the specifics of the API are different. We're hoping to have this "v2" cctz API available in the very near coming weeks (we'd hoped to have it done by next week, but that may slip a little).

Along with this new version, I do think that providing a Makefile in addition to the bazel build would be a good idea.

Regarding your question about the forum to submit questions, I think github issues are fine. We may want to setup a mailing list as well for all interested parties. Or maybe just stackoverflow would be good. I would like a better and broader communication channel for cctz users. It seems that SO is the standard for that nowadays.

JeffAbrahamson commented 8 years ago

Thanks for the update. Good to hear you're pursuing standard adoption.

Support / questions: It also depends how many users you have for cctz, which of course would grow in the event of becoming a standard. SO works well if there is enough community that questions get noticed and answered. Otherwise, mailing lists are still quite good. But there's nonetheless a level of communication (e.g., library design, bugs) where github issues and/or mailing lists are sort of the only good way to communicate.

devjgm commented 8 years ago

Hi, Jeff. A few comments, then I think we can close this issue:

We just release CCTZ v2. The new version does provide a Makefile that should help folks who are unable to use Bazel.
We also setup a mailing list for general cctz-related communication. Please join https://groups.google.com/forum/#!forum/cctz

Siesh1oo commented 8 years ago

Great Makefile! Two minor issues:

CXXFLAGS may want to have "-g" by default to create a library with debug symbols for valgrind, gdb &c.,
Please do not use "sudo" in the Makefile!

Many thanks, Kind regards

Siesh1oo commented 8 years ago

Hi,

The attached patch modifies the Makefile to add "-g" to CXXFLAGS, create install directories if not exist (possible in a PREFIX environment for local testing without polluting the main file system), allows running "make install" without sudo privilege.

A rather important fix is the "include/" prefix for CCTZ_HDRS.

The lesser annoyance of .o, .d and *.a files in "git status" is resolved by adding these patterns to .gitignore.

Please see the patch for details.

cctz-Makefile-gitignore-patch.diff.zip

devbww commented 8 years ago

Thanks Slesh1oo,

I have updated the Makefile install rules to: (1) make "sudo" optional, (2) try to create the destination directories first, and (3) VPATH expand the installation sources (so we don't need the "include/" prefix).

Not everyone will agree on compiler optimization/debug flags, so I can only suggest that you tweak your own Makefile or use something like OPT='-g -O0' from the command line.

We also suggest that you use "make -C build -f ../Makefile SRC=../" to place all the build products in the "build" subdirectory, which is already listed in .gitignore.

Bradley

Siesh1oo commented 8 years ago

Thank you!

devbww commented 8 years ago

Now that we have a Makefile, I'm going to close this issue about the bazel dependency.

Any deficiencies in the Makefile can be dealt with in a separate issue.

Thanks.

google / cctz

The bazel dependency is maybe excessive #9