Skip to content

Conversation

@hqhq
Copy link
Contributor

@hqhq hqhq commented Oct 29, 2016

So we can build runtime_tools project anyway.

Signed-off-by: Qiang Huang [email protected]

So we can build runtime_tools project anyway.

Signed-off-by: Qiang Huang <[email protected]>
@hqhq hqhq force-pushed the add_runtime_tools_link branch from 18b7cfd to a597474 Compare October 29, 2016 04:43
Makefile Outdated
rm -f $(RUNTIME_TOOLS_LINK)

$(RUNTIME_TOOLS_LINK):
ln -sfn $(CURDIR) $(RUNTIME_TOOLS_LINK)
Copy link
Contributor

Choose a reason for hiding this comment

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

-n is not in POSIX. Do we really need it to support this use-case?

@wking
Copy link
Contributor

wking commented Oct 29, 2016 via email

@hqhq hqhq force-pushed the add_runtime_tools_link branch from a597474 to 12c0532 Compare October 29, 2016 05:27
@hqhq
Copy link
Contributor Author

hqhq commented Oct 29, 2016

@wking It's ok we don't use -n, I've updated. And I know runtime-tools builds fine if I checked it out to an appropriate location in my $GOPATH, the point is users don't have to check it out to an appropriate location in their $GOPATH and they can still build runtime-tools.

It makes it more convenient like what we did in runc.

BINDIR ?= $(DESTDIR)/usr/bin

BUILDTAGS=
RUNTIME_TOOLS_LINK := $(CURDIR)/Godeps/_workspace/src/github.com/opencontainers/runtime-tools
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this approach confuse godep save, godep update and such? Maybe it would be better to setup a second tree:

LOCAL_GOPATH := $(CURDIR)/_workspace
RUNTIME_TOOLS_LINK := $(LOCAL_GOPATH)/src/github.com/opencontainers/runtime-tools
export GOPATH:=$(LOCAL_GOPATH):$(CURDIR)/Godeps/_workspace:$(GOPATH)

@mrunalp
Copy link
Contributor

mrunalp commented Nov 1, 2016

LGTM

@caniszczyk
Copy link
Contributor

caniszczyk commented Nov 3, 2016

LGTM

Approved with PullApprove

1 similar comment
@vbatts
Copy link
Member

vbatts commented Nov 3, 2016

LGTM

Approved with PullApprove

@vbatts vbatts merged commit bb04048 into opencontainers:master Nov 3, 2016
@vbatts
Copy link
Member

vbatts commented Nov 3, 2016

@caniszczyk FYI @mrunalp's LGTM did not trigger pullapprove

@wking
Copy link
Contributor

wking commented Nov 3, 2016

@mrunalp's LGTM did not trigger pullapprove

His LGTM predates #261 landing, so he wasn't cleared vs PullApprove when he made the comment.

@hqhq hqhq deleted the add_runtime_tools_link branch November 4, 2016 00:55
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.

5 participants