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

Make it easier to spin up a development environment #353

Closed
wants to merge 27 commits into from

Conversation

aszlig
Copy link
Member

@aszlig aszlig commented Jul 4, 2016

So far the instructions in the README for spinning up a development environment were quite terse and also not to its full extent, because in the end the production environment is running PostgreSQL and not SQLite.

With these changes, we now have the ability to quickly spin up a local PostgreSQL cluster within the inst/database directory which is started and shut down upon entering/exiting a nix-shell.

Using setup-dev-env we now have a function inside the Nix shell, which should do all that work on the users behalf.

The main motivation behind this change was that I usually need to be mentally prepared (like: "gosh, I need to set up a dev environment to properly work on Hydra... maybe tomorrow...") for it to go through all these steps and wanted to make it easier for new developers to dive in.

Cc: @edolstra, @domenkozar

aszlig added 17 commits July 4, 2016 18:20
Even though we currently only have build.x86_64-linux, it's probably
better not hardcode the build. We might have i686-linux builds again or
even arm builds as well in the future.

Signed-off-by: aszlig <aszlig@redmoonstudios.org>
In order to build Hydra with a different Nix version, we'd need to patch
it up within <nixpkgs>, so in order to make this easier we now have a
"nix" input attribute in release.nix which allows to specify a different
derivation like this:

nix-build release.nix \
  --arg nix '(import ~/nix/release.nix {}).build.x86_64-linux' \
  -A build.x86_64-linux

Signed-off-by: aszlig <aszlig@redmoonstudios.org>
This expands on the previous commit, so we can now do something like:

nix-shell --arg nix '(import ~/nix/release.nix {
  shell = false;
}).build.x86_64-linux'

At of the time of this commit NixOS/nix#960 isn't yet merged, so this
will fail without a way to disable nix-shell handling.

Signed-off-by: aszlig <aszlig@redmoonstudios.org>
This is for setting up a local PostgreSQL instance inside the
inst/database directory.

I'm re-using this directory because I conveniently found it inside the
.gitignore file, so let's just use that instead of adding a new path to
the .gitignore file.

The functions are currently a bit fragmented and documentation is
lacking, but in order to spin up a fully working dev server, you now
just need to do the following:

  $ nix-shell
  $ ./bootstrap
  $ ./configure $configureFlags
  $ make
  $ setup-database
  $ hydra-server

Of course, we can still improve on this, but this is at least a good
base, which also makes sure that whenever the shell exits the database
server is stopped as well.

Signed-off-by: aszlig <aszlig@redmoonstudios.org>
This makes it easier to maintain with proper syntax highlighting and
also reduces the size of release.nix.

Signed-off-by: aszlig <aszlig@redmoonstudios.org>
When looking into tests/jobs/*.sh, it seems that shell scripts are
indented with 4 spaces instead of 2, so let's make all of the shell
script files consistent in this regard.

Signed-off-by: aszlig <aszlig@redmoonstudios.org>
I've used dashed function names to make the names a bit more natural to
users of shells. So let's use CamelCased functions and variable names
for all of the internal stuff to make it more unlikely that users
accidentally call those.

Of course, everybody is still free to call them and the world will not
explode. :-)

Signed-off-by: aszlig <aszlig@redmoonstudios.org>
So, in order to get a full development environment running with support
for running hydra-server without much hassle, you can now use the
following command and it will run bootstrap, configure, make and does
the database setup for you:

nix-shell --command 'setup-dev-env; return'

If you just want to start hydra-server very very quickly it can be done
with minor modification of that command as well:

nix-shell --command 'setup-dev-env && hydra-server'

This will spin up a local webserver listening on port 3000 and if you
press CTRL+C, the web server *and* the database server will be spun
down.

Signed-off-by: aszlig <aszlig@redmoonstudios.org>
This should now be just one command and no fiddling around with
PostgreSQL or even running PostgreSQL as a system-wide service on your
local machine.

I hope this will make development for Hydra much more accessible.

Signed-off-by: aszlig <aszlig@redmoonstudios.org>
We can't use set -e here, because we need to run the shell-hook within
the context of the current shell.

So let's make sure the functions return appropriate error codes and not
simply proceed whenever there is a failure.

Signed-off-by: aszlig <aszlig@redmoonstudios.org>
The environment variables PGDATABASE, PGHOST and PGPORT are used by the
PostgreSQL client to set default connection parameters.

Now simply running "psql" will connect to the local dev database inside
inst/database.

Signed-off-by: aszlig <aszlig@redmoonstudios.org>
Very helpful for starting with development, because otherwise you have a
shiny running local Hydra instance but you can't do anything with it
because you don't have a user that is allowed to log in.

Signed-off-by: aszlig <aszlig@redmoonstudios.org>
If we're running directly with the full setup-dev-env, the setupEnvVars
function is not yet called because the database doesn't exist yet.

So let's use setupEnvVars prior to initializing the database so that all
environment variables are properly set up.

This also fixes a circular dependency on setup-dev-env, because that was
what setupEnvVars was called before and we accidentally left it in.

Signed-off-by: aszlig <aszlig@redmoonstudios.org>
Without this, whenever we press CTRL+C within the nix-shell, the
database server will get SIGINT and will terminate.

That's obviously a very unexpected situation, so let's start PostgreSQL
using the setsid command from util-linux.

Obviously that command is only available on GNU/Linux, so we make sure
the setsid command is only used whenever it is available.

Signed-off-by: aszlig <aszlig@redmoonstudios.org>
Well, it's not very much to document here, because with the set up of
the PG* variables it's way too easy (as it should be).

Signed-off-by: aszlig <aszlig@redmoonstudios.org>
This should now hopefully be the fastest and easiest way to get started
with development.

Signed-off-by: aszlig <aszlig@redmoonstudios.org>
If there is already a running PostgreSQL instance, don't bother to start
up a new one, which is what happens if you run two or more instances of
nix-shell.

Signed-off-by: aszlig <aszlig@redmoonstudios.org>
@bjornfor
Copy link

bjornfor commented Jul 5, 2016

Seems there is some churn in the shell-hook that can be squashed, but the changes look very useful. Nice!

@aszlig
Copy link
Member Author

aszlig commented Jul 5, 2016

Please don't merge yet, found a few bugs.

@bjornfor: Which churn are you referring to exactly?

@bjornfor
Copy link

bjornfor commented Jul 5, 2016

@aszlig: The setup-hook is first added inside a nix expression, then it is moved to a separate file, then it gets indentation fixes, then some error fixes... I just thought it'd be better to just have one commit "here's the setup-hook, where it should be and with proper error checking" instead of having the intermediate steps. But, it's nitpicking!

@aszlig
Copy link
Member Author

aszlig commented Jul 5, 2016

@bjornfor: Well, I prefer individual steps, so the whole commit history can be followed about why certain decisions are made instead of "here there is this thing".

Just checking whether the PID file exists is not enough, because it
might happen that PostgreSQL didn't clean up the PID file.

So, we're going to send signal 0 to the process instead to make sure
it's indeed running.

Signed-off-by: aszlig <aszlig@redmoonstudios.org>
@aszlig
Copy link
Member Author

aszlig commented Jul 5, 2016

Bug fixed, tested also against a clean working dir.

@aszlig
Copy link
Member Author

aszlig commented Jul 5, 2016

Hm, need to implement some kind of locking mechanism to make sure that if you start several shells the first one doesn't tear down the database on exit.

@aszlig aszlig changed the title Make it easier to spin up a development environment Make it easier to spin up a development environment (WIP) Jul 5, 2016
Unfortunately, the Postmaster PID file isn't just a single PID but
contains more information. So we can't just kill -0 the contents of that
file.

So let's use PostgreSQL's own pg_ctl status for this.

Signed-off-by: aszlig <aszlig@redmoonstudios.org>
Using setsid alone will run the command completely in the background, so
let's add the -w option to wait for the process to terminate/go into
background.

Signed-off-by: aszlig <aszlig@redmoonstudios.org>
That check doesn't work once we don't need to generate the
hydra-postgresql.sql file anymore.

Apart from that, the check mostly only stands in the way, because if the
setup should fail, there already is a message indicating that the file
can't be found.

Signed-off-by: aszlig <aszlig@redmoonstudios.org>
If we're going to use multiple nix-shells we don't want to spin down the
development database every time another nix-shell is closed, nor do we
want to have multiple database servers running with the same data
directory (which fails anyway).

Now the database server is started whenever we enter the first nix-shell
and it's shutdown when the last nix-shell is closed.

Signed-off-by: aszlig <aszlig@redmoonstudios.org>
@aszlig aszlig changed the title Make it easier to spin up a development environment (WIP) Make it easier to spin up a development environment Jul 7, 2016
@aszlig
Copy link
Member Author

aszlig commented Jul 7, 2016

Locking (in this case using semaphores) implemented and tested :-)

@aszlig aszlig mentioned this pull request Jul 7, 2016
bennofs and others added 5 commits July 7, 2016 16:48
This commit changes the Makefile such that any existing files are overwritten if
they are newer in the archive, so that 'make' becomes more robust.
Recent nixpkgs releases already have a hydra module, so adding another one
leads to duplicate definitions errors. This commit simply removes the custom
hydra module to fix the tests.
The current Nix master version doesn't build with Hydra and nixUnstable
doesn't work either.

This is a variation of @bennofs approach in NixOS#347, having a fixed and
known to work/build Nix version in default.nix.

While his approach moves the contents of the release.nix into the
default.nix, I'm doing it the other way around, so that we still have
release.nix solely for Hydra with the jobset input attributes
parameterizable.

To get people started as quickly as possible, the default.nix also comes
without input attributes that deal with release handling.

Having a default.nix, we no longer need shell.nix, because we can simply
test for IN_NIX_SHELL (or in this case lib.inNixShell, which does the
same at the moment).

Credits go to @bennofs for stealing parts of his comments and ripping
off his idea, thanks :-)

Signed-off-by: aszlig <aszlig@redmoonstudios.org>
Functions like powf() and roundf() are now longer available without
inclusion of <cmath> since GCC 5.4 and so the build of Hydra fails.

I presume this was a bug in older versions of GCC and these functions
shouldn't be exposed (I'm guessing they were exposed by some of the
includes used in builder.cc, but I haven't checked).

Unfortunately I haven't found anything about such a bug in the upstream
changelog or the bugtracker.

Nevertheless, we shouldn't use powf() and roundf() in any case, because
they're C99 functions:

http://en.cppreference.com/w/c/numeric/math/pow

C++11 (which we explicitly enforce using -std=c++11) on the other side
doesn't have these functions at all:

http://en.cppreference.com/w/cpp/numeric/math

So I *think* these functions shouldn't even be exposed via <cmath> and
that's probably something where GCC will catch up in future versions.

To be safe in any case, I'm now using the generalized std::pow() and
std::round() functions.

Signed-off-by: aszlig <aszlig@redmoonstudios.org>
Running hydra-server with --restart is able to detect file changes via
the inotify syscall, which is a lot better than the polling approach.

So for development (and thus within a nix-shell) this is very useful to
have in place.

Signed-off-by: aszlig <aszlig@redmoonstudios.org>
aszlig added a commit to headcounter/deployment that referenced this pull request Sep 23, 2016
I also have this in production since a while but haven't had the chance
to commit/push these changes yet.

In the next update we can probably get rid of that patch if we base this
on NixOS/hydra#353.

Signed-off-by: aszlig <aszlig@redmoonstudios.org>
@gilligan gilligan mentioned this pull request May 14, 2020
@gilligan
Copy link
Contributor

It's a bit unfortunate that so much work went into this but it still ultimately didn't lead to anything concrete - now that #759 has been merged i think I can go ahead and close this one.

@gilligan gilligan closed this May 15, 2020
@aszlig
Copy link
Member Author

aszlig commented May 16, 2020

@gilligan: Well it did work at some point back then, but it was just rotting here waiting to be reviewed/merged, but since there's now a merged solution that's even better :-)

@aszlig aszlig deleted the better-dev-environment branch May 16, 2020 00:24
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