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

Improve manual_build docs. #93

Merged
merged 3 commits into from
Oct 2, 2024

Conversation

robjh
Copy link

@robjh robjh commented Aug 6, 2024

In following the instructions to manually build Wolf recently, I found the following issues;

  • wolf required the "-fPIC" CFLAG to build correctly.
  • the outputted binary was in a different directory than the one indicated.
  • the document didn't mention docker, as a wolf runtime dependency.
  • launching wolf manually was cumbersome.

This PR aims to fix these issues.

Copy link
Member

@ABeltramo ABeltramo left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, I haven't updated that page in a long time!

Comment on lines 182 to 183
PUID="${PUID=0}" \
PGID="${PGID=0}" \
Copy link
Member

Choose a reason for hiding this comment

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

These two aren't used by Wolf, they are just used by the Docker container to setup the running user


: ${WOLF_CFG_FOLDER="config"}

XDG_RUNTIME_DIR="${XDG_RUNTIME_DIR=/tmp/sockets}" \
Copy link
Member

Choose a reason for hiding this comment

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

The format for all those env variables is odd, I thought that this was possible:

# This will set XDG_RUNTIME_DIR if it wasn't set already
${XDG_RUNTIME_DIR:=/tmp/sockets}

or

# This will override XDG_RUNTIME_DIR if not set
XDG_RUNTIME_DIR="${XDG_RUNTIME_DIR:-/tmp/sockets}"

You don't put : though and use the = which seems to work from a quick test on the CLI but I'm not sure if there's any pitfall when doing this..

Copy link
Author

Choose a reason for hiding this comment

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

I'll do some tests tonight to see what's going on. looking at the patch, it does strike me as odd syntax and i think it probably should be :=.

Copy link
Contributor

Choose a reason for hiding this comment

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

hey @robjh,

Just reading through this PR, how did the tests go?

Copy link
Author

Choose a reason for hiding this comment

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

apologies @salty2011, this fell off my radar. Thank you for the reminder, i'll get to this within the next few days. If i dont, feel free to give me a poke on discord.

Copy link
Author

Choose a reason for hiding this comment

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

okay! tests done :)
:- is the appropriate operator for those variables. I've updated the commit and rebased on the latest from the stable branch.

In following the instructions recently, I found the following issues;
- wolf required the "-fPIC" CFLAG to build correctly.
- the outputted binary was in a different directory than the one
  indicated.
- the document didn't mention docker, as a wolf runtime dependency.

- since the last commit, replace = with :- when creating the shell
  script.
@ABeltramo
Copy link
Member

Thanks for the PR and sorry this has taken me so long to finish but I wanted to add a mention to devcontainers which I think massively simplify things for us, plus you'll always automatically be on top of latest changes in our build setup/dependencies.

@ABeltramo ABeltramo merged commit 3233b32 into games-on-whales:stable Oct 2, 2024
2 of 4 checks passed
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.

3 participants