Skip to content
This repository has been archived by the owner on Jul 17, 2023. It is now read-only.

Add the dependency analyser with the go toolchain to kraft (V2) #21

Closed
wants to merge 2 commits into from

Conversation

gaulthiergain
Copy link
Member

Add the dependency analyser with the go toolchain to kraft.
This patch adds a new series of commands devel which allows
to interact with the go toolchain. For now, only the dependency
analyser has been added to the kraft tool.

This patch adds a new series of commands devel which allows
to interact with the go toolchain. For now, only the dependency
analyser has been added to the kraft tool.

The go binaries are directly integrated to the kraft repo which
allows to avoid the setup and the installation of the go runtime.

Signed-off-by: Gaulthier Gain <[email protected]>
This patch adds new entries point to the Makefile in order
to build the go toolchain as a container for the CI system.

Signed-off-by: Gaulthier Gain <[email protected]>
Copy link
Member

@nderjung nderjung left a comment

Choose a reason for hiding this comment

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

I've just checked, did you add actual binaries to the commit? I thought you were going to add the source files, .go etc?

--cache-from $(ORG)/gotools:$(GO_VERSION) \
--cache-from $(IMAGE_NAME) \
--file $(DOCKERDIR)/Dockerfile.gotools \
$(DOCKER_BUILD_EXTRA) $(DOCKERDIR)
Copy link
Member

Choose a reason for hiding this comment

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

This may have to be $(KRAFTDIR) and not $(DOCKERDIR), but I will check when I run the build myself

VOLUME [${SHARED_DIR}]

RUN set -xe
RUN chmod +x ./tools_Linux
Copy link
Member

Choose a reason for hiding this comment

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

Likely this file can be put into the Dockerfile verbatim as it won't be run outside

Copy link
Member

Choose a reason for hiding this comment

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

I've just seen this is a binary. The Dockerfile is intended to build go files?

Copy link
Member Author

Choose a reason for hiding this comment

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

What do you mean by binary? Creating the binary from sources only in the Dockerfile ?

Copy link
Member

Choose a reason for hiding this comment

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

yes

import platform
import subprocess

TOOLS_PATH = './package/docker/gotools/tools_'
Copy link
Member

Choose a reason for hiding this comment

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

This may be dependent on where how the package is installed, the extra binaries can be referenced globally, no?

Copy link
Member Author

Choose a reason for hiding this comment

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

You mean, using an absolute path?

Copy link
Member

Choose a reason for hiding this comment

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

No, the TOOLS_PATH may just need to call a method to determine where the binaries were installed on the system. I will check this.

@gaulthiergain
Copy link
Member Author

@nderjung Actually, I used the binaries for the host platform and docker. In your opinion, binaries must only be used with the host platform (without docker). Right?

@nderjung
Copy link
Member

@nderjung Actually, I used the binaries for the host platform and docker. In your opinion, binaries must only be used with the host platform (without docker). Right?

These binaries should still not be commited via git.

@nderjung
Copy link
Member

I think i can turn this thread into a draft and you can push updates here? I wonder if you're allowed to re-stage commits to this though...

@nderjung nderjung marked this pull request as draft July 30, 2020 16:03
@nderjung
Copy link
Member

nderjung commented Aug 6, 2020

Superceded (#22).

@nderjung nderjung closed this Aug 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants