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

Enable an install target #74

Closed
wants to merge 1 commit into from
Closed

Conversation

cypres
Copy link
Contributor

@cypres cypres commented Jan 24, 2016

Should work fine on Unix systems, and support install prefixes.
Installs both a static and a shared library to the same location, so we get an .a and a .so/.dylib file, which can be linked to in the ordinary way, ie. -lg3logger.

As discussed in #49

More work could be done, but for now I think we need some basic support added.

Should work fine on Unix systems, and support install prefixes.
Installs both a static and a shared library to the same location, so we
get an .a and a .so/.dylib file, which can be linked to in the ordinary
way, ie. `-lg3logger`.
# Check the output result and install accordingly.
# ==========================================================================
# INCLUDE (${g3log_SOURCE_DIR}/CPackLists.txt)
INCLUDE (${g3log_SOURCE_DIR}/CPackLists.txt)
Copy link
Owner

Choose a reason for hiding this comment

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

please see comment in CPackLists.txt

Copy link
Owner

Choose a reason for hiding this comment

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

to be fixed in a coming pull request

Copy link
Owner

Choose a reason for hiding this comment

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

#83

@KjellKod
Copy link
Owner

Awesome work @duedal

@KjellKod
Copy link
Owner

FYI :make install worked great on linux

@KjellKod
Copy link
Owner

KjellKod commented Feb 2, 2016

@duedal please make a comment when this is ready to be re-reviewed.

@KjellKod
Copy link
Owner

@duedal did you see my review comments?

@cypres
Copy link
Contributor Author

cypres commented Feb 17, 2016

@KjellKod I did yes, things just been a bit crazy at work so havn't had the time to make the required changes yet. 😔 I'm going to work on this during the weekend.

We need to make sure we don't break anything on windows for sure. I don't know about make package on OS X, I don't see the need for it since OS X users simply do brew install g3log anyways, and the install target allows us to optimize the homebrew formula.

@KjellKod
Copy link
Owner

No worries.

If it's too hectic right now I can always make a pull request to your pull request.

You can review it and if all is OK you merge it to your branch "install-target" and it will show up here (on this pull request).

It sounds complicated but is really straight forward.

@KjellKod
Copy link
Owner

@duedal brew install g3log,. Hah! Sweet. I had no idea until you told me

@KjellKod
Copy link
Owner

@duedal, aha and you are the culprit too ;)

Homebrew/legacy-homebrew@a8114e1#Library/Formula/g3log.rb

Thanks man!

@KjellKod
Copy link
Owner

KjellKod commented Mar 6, 2016

taken over by pull request: #83 to address the comments above

@KjellKod KjellKod closed this Mar 6, 2016
@KjellKod
Copy link
Owner

KjellKod commented Mar 7, 2016

@duedal, please see release 1.2 of g3log
https://github.com/KjellKod/g3log/releases/tag/1.2

What is needed to update the OSX brew install with this?

Contact Information
Kjell Hedström
Mobile: +1 - 617.378.1915

E-mail: Hedstrom@KjellKod.cc

On Wed, Feb 17, 2016 at 1:41 AM, Hans Duedal notifications@github.com
wrote:

@KjellKod https://github.com/KjellKod I did yes, things just been a bit
crazy at work so havn't had the time to make the required changes yet. [image:
😔] I'm going to work on this during the weekend.

We need to make sure we don't break anything on windows for sure. I don't
know about make package on OS X, I don't see the need for it since OS X
users simply do brew install g3log anyways, and the install target allows
us to optimize the homebrew formula.


Reply to this email directly or view it on GitHub
#74 (comment).

@cypres
Copy link
Contributor Author

cypres commented Mar 8, 2016

Hi. Sorry I haven't been able to commit the time to work with CPack.

For the next release, could you enable CPack on OS X too? Something like.

IF(${CMAKE_SYSTEM_NAME} MATCHES "Linux" OR ${CMAKE_SYSTEM_NAME} MATCHES "Darwin")
   INCLUDE (${g3log_SOURCE_DIR}/CPackLists.txt)
ENDIF()

@cypres cypres deleted the install-target branch March 8, 2016 09:41
@cypres
Copy link
Contributor Author

cypres commented Mar 8, 2016

Sent a PR to homebrew with the 1.2 release.
Homebrew/legacy-homebrew#49870

@KjellKod
Copy link
Owner

KjellKod commented Mar 8, 2016

I see the PR was closed and not merged (?). Anything I can do to help? What went wrong?

Sent from my iPhone

On Mar 8, 2016, at 2:42 AM, Hans Duedal notifications@github.com wrote:

Sent a PR to homebrew with the 1.2 release.
Homebrew/legacy-homebrew#49870


Reply to this email directly or view it on GitHub.

@cypres
Copy link
Contributor Author

cypres commented Mar 8, 2016

They do signed commits, it was "merged" in this commit: Homebrew/legacy-homebrew@39974fd

@KjellKod
Copy link
Owner

KjellKod commented Mar 8, 2016

OK. Cool. Thanks

Contact Information
Kjell Hedström
Mobile: +1 - 617.378.1915

E-mail: Hedstrom@KjellKod.cc

On Tue, Mar 8, 2016 at 7:43 AM, Hans Duedal notifications@github.com
wrote:

They do signed commits, it was "merged" in this commit: Homebrew/homebrew@
39974fd
Homebrew/legacy-homebrew@39974fd


Reply to this email directly or view it on GitHub
#74 (comment).

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