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

makefile: add 3 PHONY targets #3483

Merged

Conversation

justinsb
Copy link
Member

Marking some targets as phony that are plainly missing dependencies.

Marking some targets as phony that are plainly missing dependencies.
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Sep 30, 2017
@chrislovecnm
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 30, 2017
@k8s-github-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chrislovecnm

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@k8s-github-robot k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 30, 2017
@justinsb
Copy link
Member Author

/retest

@k8s-github-robot
Copy link

/test all [submit-queue is verifying that this PR is safe to merge]

@chrislovecnm
Copy link
Contributor

@alrs btw

I1001 00:27:29.636] /bin/sh: 1: protoc: not found
I1001 00:27:29.636] Makefile:196: recipe for target 'protokube/pkg/gossip/mesh/mesh.pb.go' failed
I1001 00:27:29.637] make: *** [protokube/pkg/gossip/mesh/mesh.pb.go] Error 127
I1001 00:27:29.929] Makefile:360: recipe for target 'protokube-build-in-docker' failed

With adding the missing PHONY targes we are recreating the problem we saw in Travis

@chrislovecnm
Copy link
Contributor

I have not idea how this is happenning, maybe docker version??? I am not able to recreate this locally.

@chrislovecnm
Copy link
Contributor

@alrs
Copy link
Contributor

alrs commented Oct 1, 2017

None of these three targets are PHONY, so I don't recommend doing this.

I'm making some progress in my own refactoring of the Makefile to deal with this issue, would you like me to put up a WIP so I can show you what I'm up to?

@alrs
Copy link
Contributor

alrs commented Oct 1, 2017

We know why this is happening: the Makefile thinks that protokube/pkg/gossip/mesh/mesh.pb.go either doesn't exist or is out-of-date, so it tries to rebuild it.

This could be because of:

  1. Filesystem times are incorrect and make thinks that something is out-of-date when it isn't
  2. make is being run outside of the root of the project, and wherever it goes looking for 'protokube/pkg/gossip/mesh/mesh.pb.go' it fails to find it, so it attempts to build it, which it can't, because there is no protoc.

@alrs
Copy link
Contributor

alrs commented Oct 1, 2017

It's really tough in this Makefile to figure out when things are happening on the host, in a container, and from which directory. I'm trying to refactor the Makefile so that make is only invoked from the host, the docker containers are set to run with /go/src/k8s.io/kops as the pwd.

@alrs
Copy link
Contributor

alrs commented Oct 1, 2017

Here's a wip-branch that is apropos to this conversation: #3505

@k8s-github-robot
Copy link

/test all [submit-queue is verifying that this PR is safe to merge]

@k8s-github-robot
Copy link

Automatic merge from submit-queue.

@k8s-github-robot k8s-github-robot merged commit 53ec69f into kubernetes:master Oct 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants