-
-
Notifications
You must be signed in to change notification settings - Fork 14.6k
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
soldat-unstable: init at unstable-2020-11-26; gamenetworkingsockets: init at 1.2.0 #108436
Conversation
ce3d1ff
to
e514dec
Compare
# make sure soldat{,server} find their game archive, | ||
# let them write their state and configuration files | ||
# to $XDG_CONFIG_HOME/soldat/soldat{,server} unless | ||
# the user specifies otherwise. | ||
for p in $out/bin/soldatserver $out/bin/soldat; do | ||
configDir="\''${XDG_CONFIG_HOME:-\$HOME/.config}/soldat/$(basename "$p")" | ||
|
||
wrapProgram "$p" \ | ||
--run "mkdir -p \"$configDir\"" \ | ||
--add-flags "-fs_portable 0" \ | ||
--add-flags "-fs_userpath \"$configDir\"" \ | ||
--add-flags "-fs_basepath \"${base}/share/soldat\"" | ||
done | ||
''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What’s the default paths soldat uses?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can upstream XDG_CONFIG_HOME instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
soldat and soldatserver expect to be run from the client
or server
subdirectories of the development repository respectively. By default they assume fs_basepath
and fs_userpath
are the same directory as their executable which obviously doesn't work since that would be in /nix/store
.
I contemplated using ./
as a default for fs_userpath
, but it's also not a nice solution and relatively annoying.
Upstreaming XDG_CONFIG_HOME
could be an idea, unfortunately haven't heard a lot on the other patch yet as well. I guess I'll try make a patch which uses XDG_CONFIG_HOME
on Unix and %AppData%
on Windows…
This is a semi-automatic executed nixpkgs-review which does not build all packages (e.g. lumo, tensorflow or pytorch) Result of 2 packages built:
|
This is a semi-automatic executed nixpkgs-review which does not build all packages (e.g. lumo, tensorflow or pytorch) Result of 1 package failed to build and are new build failure:
|
log not found |
This is a semi-automatic executed nixpkgs-review which does not build all packages (e.g. lumo, tensorflow or pytorch) Result of 2 packages built:
Not sure why it now builds... |
Shall we merge? |
Fixing |
This is a semi-automatic executed nixpkgs-review which does not build all packages (e.g. lumo, tensorflow or pytorch) Result of 1 package failed to build and are new build failure:
|
e514dec
to
4b2b51f
Compare
Okay, this is a genuine issue with GameNetworkingSockets, |
4b2b51f
to
b0bc84d
Compare
b0bc84d
to
6440b8d
Compare
The build failure is already resolved on |
Result of 1 package marked as broken and skipped:
|
This is a semi-automatic executed nixpkgs-review which does not build all packages (e.g. lumo, tensorflow or pytorch) Result of 2 packages built:
|
cmakeFlags = [ "-G Ninja" ]; | ||
|
||
# tmp home for go | ||
preBuild = "export HOME=\"$TMPDIR\""; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
preBuild = "export HOME=\"$TMPDIR\""; | |
preBuild = '' | |
export HOME=$TMPDIR | |
''; |
@SuperSandro2000 Automating review is nice and all, but please scroll through the backlog and tell me it’s not starting to get spammy. e.g. your bot (‽) added the same comment as earlier, even though @sternenseemann already said that the nit is irrelevant. We shouldn’t force stylistic nitpicks onto maintainers that know what they are doing. |
I am not a bot. I have several humans that can prove that I made out of flesh. Jokes aside. I read through the diff and thought the second time that this is still not quite right. Generally variables which contain commands use '' because they can easily be multiline even if they only contain one command. Additionally some extra escaping is required here because of that. I think that adds the false intention that $TMP maybe shouldn't be evaluated at that point and only at runtime. I try to not nit about older packages to much when they just change one line or do a simple version update but for new packages or packages with bigger changes we should focus on making them clean and use hooks and variables when available. Especially for new packages we should really think how to make them clean, short and simple to easily maintain them in the future, make updates easy and don't hinder treewide changes with unusual syntaxes or escapes.
We already enforce a pretty great list of stylistics and probably some of them are nitpicks but so far I don't think I have seen one that is literally useless. |
Cool, merging. |
It makes a difference of one `\n in the string and I'd argue in this case it reduces noise and readability since a relatively unimportant compatibility fix is not occupying to much space. Also I think I'll be fine maintaining this whereas endless rebasing is just plain annoying. |
@Profpatsch If you wouldn't be the first person labeling my manual work as a bot I would be pretty offended by that. @sternenseemann You could have just accepted that change but instead we are arguing about something which is not worth to argue about. I am not writing comments on things because I feel like I want to annoy but I think they would improve the overall quality by more than a \n. |
Motivation for this change
Adds the open sourced development version of soldat, a 2d side
scrolling shooter. The soldat-unstable attribute name was chosen since
this is a development version of the unfinished soldat 1.8 version which
most notably has incompatible netcode to the windows-only soldat 1.7
version which you can download on steam for example, so naming this
soldat-unstable hopefully avoids confusion as to why it is not possible
to connect to soldat 1.7 servers.
We wrap the soldat and soldatserver executables to have fixed paths set
for fs_basepath (the game archive) and fs_userpath (configuration, state
files). The latter is set to
$XDG_CONFIG_HOME/soldat/soldat{,server}
bydefault to stop the executables from polluting the working directory.
Both settings can be overriden however by giving the respective options
on the command line of the wrapper.
GameNetworkingSockets is a dependency of soldat.
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)