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

Some functionality and fixes from CDI's repository #3

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

gammafunk
Copy link
Member

This branch is based off current master and has cherry-picked and cleaned up commits from CDI's local repository implementing various fixes and pieces of functionality. The commit comments explain motivation and any important implementation details, but there are a few that might need discussion before we'd merge them:

  • 27690d8 A cleanup to the git installer to remove the built binary from the source tree after installation. Necessary to save disk space over time, but maybe there's a cleaner way.
  • 7b3e905 Do a git fetch and git reset --hard to the relevant branch when updating the repository. This also requires a change to the stable installer to remove the existing data dir, and this change is propagated to the relevant installers I add in subsequent commits. I don't think there's a downside to this for non-experimental installs, but maybe there are issues or there's a better approach.
  • 6fdcf59 Don't publish default RC or macro files. This didn't seem like functionality we need to support, and we have stale files in the repo. This commit makes the necessary changes a bit more systematically than what I actually use on CDI, so I will test this more when I convert CDI to use this branch in its dgamelaunch-config.
  • 6edef29 This is somewhat complicated functionality likely only useful to me. I use it to run qw on CDI. Still, it's added in a way that shouldn't inconvenience any user of this repo. It's also easier maintenance-wise if I have it in master and nice to have other people looking it over.

The wizard user and updates to experimental branch support shouldn't be problematic, but of course benefit from being looked over as well.

In the launchers, don't attempt to add the -wizard command line argument
for dgl admins if -wizard is already present. This allows game
configurations to always specify -wizard without breaking game launching
for admins.
Zot Defense has been removed for a long time, and these directories
aren't needed.
Remove the original copy of the recently built trunk binary from the
build dir. Otherwise these 200MB files simply accrue, filling up disk
space. They aren't needed after installation is finished, since the
binary is copied into the chroot.
Currently we do a `git pull` to update the repository, which of course
doesn't work in the case where the branch has been force pushed. Force
pushes shouldn't happen (hopefully) for stable or trunk, but they're
common for experimental branches. This commit has the repo update do a
`git fetch` followed by a hard reset to the relevant branch. It
additionally updates the stable installer to remove the data directory
with each installation. This way we'll not have stale data files lying
around and potentially loaded by crawl if they get removed from the
repository by a previous commit.
This commit adds commands to manage a wizards user group for dgl who
have the ability to use Wizard mode without getting other associated
admin rights. The launchers now add the `-wizard` argument whenever a
user belongs to this group. Admins continue to get this argument as
well.
The dgl scripts try to publish RC and macro files stored in this repo to
allow servers to have custom defaults. I don't believe any DGL servers
use this functionality, and it results in publishing potentially stale
files. Simply removing the in-repo files will cause errors with the
publishing script, so this commit also removes support for publishing
custom RC and macro files in the script.

The existing dgamealaunch config already uses the RC files installed by
the installer rather than those published by the dgl scripts. For
macros, we tweak the commands for resetting options to simply unlink the
user's macro file instead of copying a default file.
Previously we updated the stable branch scripts to allow them to work
with experimentals, but this had unwanted side effects. Directories were
created that experimentals don't use, some of which mangled the branch
name into a numeric version, and there was no support for using a game
name different from the git branch name. The latter is important since
developers like to use funny names that might confuse users in e.g.
WebTiles lobby listings.

This commit adds a set of scripts specifically for experimentals: a
build updater, an installer, and a launcher. The updater require two
arguments, a git branch name used for git purposes, and a game name used
in naming the crawl binary and any relevant directories.  The installer
and launcher only expect the game name, since that's the only value they
need. The installer doesn't create sprint or tutorial directories, since
we don't use those for experimentals. The launcher uses the user's git
RC file for its RC.

We also revert changes to the stable updater that relaxed conditions on
the branch name. This script goes back to being specialized for the
stable DCSS release branches.
Don't use `echo -e`, using printf instead if we need to control
newlines. Also fix up existing printf usage.
This current arguments don't match the bash builtin read argument
syntax.
For running bots like qw, its helpful to have installs of stable and
trunk that are accessible only to bot accounts. This way qw can run on
an untracked server like CDI, but the bot-only logfiles and milestone
files can still be added to Sequell, Tournament, and CAO scoring.  In
the future, bots might additionally need slightly different game build
options.

This commit adds a bot user flag (value 64) and updaters, installers,
and launchers for bot-only installs of stable and trunk. The launcher
refuses to execute crawl if the user does not have the bot flag,
providing an appropriate message in both dgamelaunch and webtiles when
this happens. When running the bot stable/git updaters along with the
regular updaters, a proper ccache setup will ensure that the bot builds
are very fast. Both installers do the same type of install as the
regular stable installer, namely games are always forced transfered.
Bots don't need to decide on a per-commit basis whether to transfer a
trunk game, and not maintaining an additional set of per-commit builds
of trunk simplifies management.

Since this functionality is likely only useful to myself and CDI, I'm
not adding any configuration examples to dgamelaunch.conf or configy.py.
Copy link
Member

@rawlins rawlins left a comment

Choose a reason for hiding this comment

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

Some small comments -- seems worth merging most of this.

Re the bot scripts in the last commit: along the lines of what the commit message says, I'm unsure if it's a good idea to merge this; one worry is that it has a lot of duplicated code that will likely go stale over time. (I wonder if there's a way to implement the feature that doesn't have so many bot-specific variants of existing scripts?) But if this doesn't get merged we should reserve the bot flag somehow in both dgl-config and in webtiles.

BINARY_NAME="$CRAWL_BINARY_PATH/$BINARY_BASE_NAME"
GAME_FOLDER="$CRAWL_GIT_DIR/$BINARY_BASE_NAME"

if user-is-admin && [[ $* != *-wizard* ]]; then
if ( user-is-admin || user-is-wizard ) && [[ $* != *-wizard* ]]; then
Copy link
Member

Choose a reason for hiding this comment

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

rather than having two calls with separate queries here, I think it would be better for user-is-wizard to check both flags in a single query

@@ -287,7 +294,7 @@ fi
BINARY_NAME="$CRAWL_BINARY_PATH/$BINARY_BASE_NAME-$OUR_GAME_HASH"
GAME_FOLDER="$CRAWL_GIT_DIR/$BINARY_BASE_NAME-$OUR_GAME_HASH"

if user-is-admin && [[ $* != *-wizard* ]]; then
if ( user-is-admin || user-is-wizard ) && [[ $* != *-wizard* ]]; then
Copy link
Member

Choose a reason for hiding this comment

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

see comment for crawl-stable-launcher.sh

@@ -20,7 +20,8 @@ update-crawl-ref() {
say "Updating git repository $REPO_DIR"
( cd $REPO_DIR && git checkout -f &&
git checkout $BRANCH &&
git pull )
git fetch &&
git reset --hard FETCH_HEAD )
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason to use a hard reset over git pull --rebase? I think they should be the same for our normal use case, and also handle if someone has a local commit of some kind (which regular servers shouldn't do), though I haven't tested this.

refracta referenced this pull request in refracta/dcss-server May 20, 2024
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.

2 participants