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

Update building documentation for macOS #4850

Conversation

jossemargt-3pillar
Copy link
Contributor

Signed-off-by: Jonnatan Jossemar Cordero jonnatan.cordero@suse.com

Proposed Changes

The k3s build scripts rely on GNU utils, like sed, so when these scripts are run on a macOS environment they don't work as expected.

Update the documentation to point out this scenario and suggest the reader to spin up the Linux virtual machine defined on the Vagrantfile within this project.

Types of Changes

Documentation

Verification

Linked Issues

User-Facing Change


Further Comments

@jossemargt-3pillar jossemargt-3pillar requested a review from a team as a code owner December 29, 2021 21:27
BUILDING.md Outdated

The source code for Kubernetes is in `vendor/` and the location from which that is copied
is in `./go.mod`. Go to the referenced repo/tag and you'll find all the patches applied
is in [go.mod](go.mod). Go to the referenced repo/tag and you'll find all the patches applied
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since the vendor/ directory has been removed, I don't think this is accurate anymore. What should we say instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd remove.

```bash
SKIP_VALIDATE=true make
```

If you make any changes to go.mod and want to update the vendored modules, you should run the following before running `make`:
In case you make any changes to [go.mod](go.mod), you should run `go mod tidy` before running `make`.
Copy link
Contributor

Choose a reason for hiding this comment

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

We can run make deps here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I can make that change. But make deps internally does go mod vendor, and I was under the impression that we are stirring away from vendor/ directory. So, should I remove go mod vendor from the Makefile or add vendor/ to the .gitignore?

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's get @luthermonson to weigh-in here.

Copy link
Contributor

Choose a reason for hiding this comment

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

We should remove the make deps from the Makefile entirely, we do not want to go mod vendor anymore, and then the command is just a wrapper for go mod tidy

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree with @dereknola, however updating the Makefile or .gitignore might be outside of this PR scope, and the documentation is still correct if we omit to use that make target.

Either way, I'm open to do what the team sees proper.

BUILDING.md Outdated
## Build k3s from source

Before getting started, bear in mind that this repository includes all of
Kubernetes history, so consider shallow clone it (`--depth 1`) to speed up the
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Kubernetes history, so consider shallow clone it (`--depth 1`) to speed up the
Kubernetes history, so consider shallow cloning with (`--depth 1`) to speed up the

BUILDING.md Outdated
on GNU utils (i.e., `sed`),
[which slightly differ on macOS](https://unix.stackexchange.com/a/79357). So, if
you need to build k3s on a macOS environment, it is suggested to use the virtual
machine defined on this repository [Vagrantfile](Vagrantfile) to perform the
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
machine defined on this repository [Vagrantfile](Vagrantfile) to perform the
machine defined on this repository's [Vagrantfile](Vagrantfile) to perform the

@briandowns
Copy link
Contributor

Is there any reason why we're manually handling line breaks and not letting Markdown perform the line wrapping for us?

@jossemargt-3pillar
Copy link
Contributor Author

jossemargt-3pillar commented Jan 3, 2022

Is there any reason why we're manually handling line breaks and not letting Markdown perform the line wrapping for us?

My IDE does it for me, it is to maintain the MD documents with a 80 characters long margin. I can re-join them if we prefer the other way.

PS: No manual labor I promise.

@briandowns
Copy link
Contributor

Is there any reason why we're manually handling line breaks and not letting Markdown perform the line wrapping for us?

My IDE does it for me, it is to maintain the MD documents with a 80 characters long margin. I can re-join them if we prefer the other way.

PS: No manual labor I promise.

Yeah, let's let the browser handle that otherwise, for consistency's sake, we all then need that ide setting.

Problem: The k3s build scripts rely on GNU utils, like sed, so when
these scripts are run on a macOS environment they don't work as
expected.

Solution: Update the documentation to point out this scenario and
suggest the reader to spin up the Linux virtual machine defined on
the Vagrantfile within this project.

Signed-off-by: Jonnatan Jossemar Cordero <jonnatan.cordero@suse.com>
Signed-off-by: Jonnatan Jossemar Cordero <jonnatan.cordero@suse.com>
@jossemargt-3pillar jossemargt-3pillar merged commit a3190bd into k3s-io:master Jan 10, 2022
@jossemargt-3pillar jossemargt-3pillar deleted the documenation/update-building-docs branch January 10, 2022 16:46
rbrtbnfgl pushed a commit to rbrtbnfgl/k3s that referenced this pull request Jan 14, 2022
Update building documentation for macOS

Problem: The k3s build scripts rely on GNU utils, like sed, so when
these scripts are run on a macOS environment they don't work as
expected.

Solution: Update the documentation to point out this scenario and
suggest the reader to spin up the Linux virtual machine defined on
the Vagrantfile within this project.

Signed-off-by: Jonnatan Jossemar Cordero <jonnatan.cordero@suse.com>
Signed-off-by: Roberto Bonafiglia <roberto.bonafiglia@gmail.com>
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

4 participants