mirror of
https://github.com/projectsend/projectsend.git
synced 2026-10-03 12:54:18 +00:00
Merge pull request #1670 from projectsend/fix/1661-update-instructions-for-source-builds
Tell a clone-and-build install to rebuild, not to pull
This commit is contained in:
+13
-1
@@ -15,6 +15,17 @@ a version is cut.
|
||||
|
||||
### Fixed
|
||||
|
||||
- **An installation that builds its own containers is no longer told to pull.** ProjectSend prints
|
||||
the update instructions for the way you installed it, and it had two answers where it needed
|
||||
three: anything running in a container was handed `docker compose pull && docker compose up -d`,
|
||||
including the Compose stack that builds from a checkout of the repository. There is no image
|
||||
behind those containers to pull, so both commands ran, reported success and changed nothing — and
|
||||
the dashboard went on offering the same release. Those installations are now recognised and given
|
||||
`git pull && docker compose up -d --build` instead, with the two extra steps a checkout needs when
|
||||
a release moves its dependencies or its frontend.
|
||||
([#1661](https://github.com/projectsend/projectsend/issues/1661), reported by
|
||||
[@mueller7382](https://github.com/mueller7382))
|
||||
|
||||
- **The dashboard no longer fails on shared hosting.** To decide which update instructions to print,
|
||||
ProjectSend asks whether it is running inside a container by looking for a file in the root of the
|
||||
filesystem. On shared hosting PHP is usually confined to your own directory, and looking outside it
|
||||
@@ -23,7 +34,8 @@ a version is cut.
|
||||
it always was: a server that keeps PHP inside a single directory is not our container image, and
|
||||
gets the manual update instructions, which is correct for shared hosting anyway. Nothing to change
|
||||
on your side, and no setting you would have been able to change if there were.
|
||||
([#1663](https://github.com/projectsend/projectsend/issues/1663))
|
||||
([#1663](https://github.com/projectsend/projectsend/issues/1663), reported by
|
||||
[@denkfabrik-li](https://github.com/denkfabrik-li))
|
||||
|
||||
## 2.1.0 — 18 August 2026
|
||||
|
||||
|
||||
@@ -5,35 +5,56 @@ declare(strict_types=1);
|
||||
namespace App\Modules\Platform\Installation;
|
||||
|
||||
/**
|
||||
* Whether this installation runs from a container image or from files on a
|
||||
* server somebody administers directly.
|
||||
* Whether this installation runs from a container image, from a container
|
||||
* the operator builds themselves, or from files on a server somebody
|
||||
* administers directly.
|
||||
*
|
||||
* It exists because the application tells administrators how to upgrade, and
|
||||
* the two answers have nothing in common. A container is replaced —
|
||||
* `docker compose pull && docker compose up -d`, with the entrypoint running
|
||||
* the migrations on the way up. A manual install is a sequence somebody
|
||||
* performs by hand: back up, take the site down, unpack the release over the
|
||||
* directory, migrate, refresh the caches, bring it back (INSTALL.md).
|
||||
* the three answers have nothing in common. A published container is
|
||||
* replaced — `docker compose pull && docker compose up -d`, with the
|
||||
* entrypoint running the migrations on the way up. A container built from a
|
||||
* checkout has to be given new code and rebuilt. A manual install is a
|
||||
* sequence somebody performs by hand: back up, take the site down, unpack
|
||||
* the release over the directory, migrate, refresh the caches, bring it back
|
||||
* (INSTALL.md).
|
||||
*
|
||||
* Printing the container command to someone who installed from a zip is
|
||||
* worse than printing nothing: it names a tool they do not have, for a stack
|
||||
* they are not running, at the exact moment they are trying to do the right
|
||||
* thing. That was the behaviour before this class existed — the command was
|
||||
* a hardcoded string in two React components, written when Docker was the
|
||||
* only supported path.
|
||||
* Printing the wrong one of those is worse than printing nothing. For a
|
||||
* manual install the container command names a tool they do not have, for a
|
||||
* stack they are not running, at the exact moment they are trying to do the
|
||||
* right thing — that was the behaviour before this class existed, when the
|
||||
* command was a hardcoded string in two React components. For a stack built
|
||||
* from a checkout it is worse still, because the command runs: `pull` skips
|
||||
* services that have no image to pull and `up -d` then finds every container
|
||||
* already current, so the update reports success and changes nothing, and
|
||||
* the dashboard goes on offering the same release forever (#1661).
|
||||
*
|
||||
* The detection is the presence of the file a container runtime leaves in
|
||||
* the root filesystem. It is a deliberately conservative signal: something
|
||||
* exotic enough to run neither Docker nor Podman is reported as a manual
|
||||
* install, which is the safer wrong answer of the two — the manual
|
||||
* instructions are steps a person follows and check for themselves, while
|
||||
* the container command is one they would paste.
|
||||
* Two signals, in order:
|
||||
*
|
||||
* 1. The published image sets PROJECTSEND_IMAGE. A positive marker set at
|
||||
* build time is the only one a bind mount can neither forge nor hide.
|
||||
* 2. Failing that — images published before that variable existed — a
|
||||
* working tree in the install directory. The image is built from an
|
||||
* unpacked release artifact and has none; the Compose stack in the
|
||||
* repository bind-mounts the repository itself.
|
||||
*
|
||||
* Being in a container at all is the presence of the file a container
|
||||
* runtime leaves in the root filesystem. It is a deliberately conservative
|
||||
* signal: something exotic enough to run neither Docker nor Podman is
|
||||
* reported as a manual install, which is the safer wrong answer of the
|
||||
* three — the manual instructions are steps a person follows and checks for
|
||||
* themselves, while the container commands are ones they would paste.
|
||||
*/
|
||||
class Installation
|
||||
{
|
||||
public function kind(): InstallationKind
|
||||
{
|
||||
return $this->inContainer() ? InstallationKind::Container : InstallationKind::Manual;
|
||||
if (! $this->inContainer()) {
|
||||
return InstallationKind::Manual;
|
||||
}
|
||||
|
||||
return $this->builtFromSource()
|
||||
? InstallationKind::ContainerSource
|
||||
: InstallationKind::Container;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -54,4 +75,22 @@ class Installation
|
||||
// directory is not the container image.
|
||||
return @file_exists('/.dockerenv') || @file_exists('/run/.containerenv');
|
||||
}
|
||||
|
||||
/**
|
||||
* Protected for the same reason as inContainer(), and answered the same
|
||||
* way in tests.
|
||||
*/
|
||||
protected function builtFromSource(): bool
|
||||
{
|
||||
// getenv() rather than env(): once the configuration is cached,
|
||||
// env() outside a config file returns null, and the answer would
|
||||
// silently flip on the installs most likely to have cached it.
|
||||
if (getenv('PROJECTSEND_IMAGE') === '1') {
|
||||
return false;
|
||||
}
|
||||
|
||||
// A worktree checkout writes .git as a file rather than a
|
||||
// directory, so ask whether it exists, not what it is.
|
||||
return file_exists(base_path('.git'));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -11,9 +11,17 @@ namespace App\Modules\Platform\Installation;
|
||||
*/
|
||||
enum InstallationKind: string
|
||||
{
|
||||
/** Runs from an image: upgrading is pulling a new one. */
|
||||
/** Runs from the published image: upgrading is pulling a new one. */
|
||||
case Container = 'container';
|
||||
|
||||
/**
|
||||
* Runs from a container the operator builds themselves, out of a
|
||||
* checkout of the repository: upgrading is new code first, then a
|
||||
* rebuild. Pulling does nothing here — there is no published image
|
||||
* behind these containers to pull.
|
||||
*/
|
||||
case ContainerSource = 'container-source';
|
||||
|
||||
/** Runs from files on a server someone administers: upgrading is INSTALL.md's sequence. */
|
||||
case Manual = 'manual';
|
||||
}
|
||||
|
||||
@@ -126,6 +126,14 @@ RUN mkdir -p storage/app/files storage/framework/cache storage/framework/session
|
||||
# session and makes every encrypted column unreadable.
|
||||
VOLUME ["/var/www/html/storage"]
|
||||
|
||||
# How the application knows it is this image and not a container somebody
|
||||
# built from a checkout — the two upgrade completely differently, and it is
|
||||
# the application that prints the instructions. See Installation. It has to
|
||||
# be set here rather than inferred at runtime: an operator who bind-mounts
|
||||
# over /var/www/html can hide any file the image ships as its evidence, and
|
||||
# an environment variable survives that.
|
||||
ENV PROJECTSEND_IMAGE=1
|
||||
|
||||
EXPOSE 80
|
||||
|
||||
# Laravel's health route (bootstrap/app.php: health: '/up'). Hitting it
|
||||
|
||||
@@ -2,6 +2,7 @@ import { usePage } from '@inertiajs/react';
|
||||
import { AlertTriangle } from 'lucide-react';
|
||||
|
||||
import { Alert, AlertDescription, AlertTitle } from '@/components/ui/alert';
|
||||
import { isContainerInstall } from '@/components/update-instructions';
|
||||
import { useTranslation } from '@/hooks/use-translation';
|
||||
import { type SharedData } from '@/types';
|
||||
|
||||
@@ -27,7 +28,10 @@ export function CodeNoticeBanner() {
|
||||
return null;
|
||||
}
|
||||
|
||||
const container = notice.install_kind === 'container';
|
||||
// Both container kinds answer this the same way: what fixes it is
|
||||
// recreating the container, whether its image came from a registry or
|
||||
// from a build on this machine.
|
||||
const container = isContainerInstall(notice.install_kind);
|
||||
|
||||
// Stated as a fact first, cause second. A deliberate rollback lands
|
||||
// here too, and telling somebody to reload PHP-FPM when they meant to
|
||||
|
||||
@@ -2,7 +2,17 @@ import { UpdateOptionsDialog } from '@/components/update-options-dialog';
|
||||
import { useTranslation } from '@/hooks/use-translation';
|
||||
import { cn } from '@/lib/utils';
|
||||
|
||||
export type InstallKind = 'container' | 'manual';
|
||||
export type InstallKind = 'container' | 'container-source' | 'manual';
|
||||
|
||||
/**
|
||||
* Whether this installation runs in a container, whichever way it got
|
||||
* there. Everything that is about the runtime — restarting it, recreating
|
||||
* it — is the same answer for both container kinds; only the upgrade
|
||||
* itself differs.
|
||||
*/
|
||||
export function isContainerInstall(kind: InstallKind): boolean {
|
||||
return kind === 'container' || kind === 'container-source';
|
||||
}
|
||||
|
||||
/**
|
||||
* How to actually apply an available update, for this server.
|
||||
@@ -15,6 +25,13 @@ export type InstallKind = 'container' | 'manual';
|
||||
* have, for a stack they are not using, at the exact moment they were trying
|
||||
* to do the right thing.
|
||||
*
|
||||
* The same string was wrong a second way, for containers this time: a stack
|
||||
* built from a checkout has no image to pull, so both commands succeed,
|
||||
* report success, and change nothing at all. That install then keeps being
|
||||
* offered the same release for as long as its operator keeps following the
|
||||
* instructions on this screen (#1661) — which is why 'container' and
|
||||
* 'container-source' are told different things.
|
||||
*
|
||||
* `compact` is for the dashboard card, a narrow column beside other widgets.
|
||||
* `codeClassName` exists for the same caller — its code sits inside a warning
|
||||
* alert and has to match it.
|
||||
@@ -44,6 +61,24 @@ export function UpdateInstructions({
|
||||
);
|
||||
}
|
||||
|
||||
if (kind === 'container-source') {
|
||||
return (
|
||||
<>
|
||||
{/* On its own line rather than inline like the image's one-liner:
|
||||
this command is long enough to wrap inside the dashboard's
|
||||
narrow column, and a command split across two lines mid-word
|
||||
is one somebody retypes wrongly. */}
|
||||
<p className="text-muted-foreground mb-1">{t('To update, run this in the directory you cloned:')}</p>
|
||||
<code className={cn('block rounded px-2 py-1.5 break-words', codeClassName ?? 'bg-muted')}>git pull && docker compose up -d --build</code>
|
||||
<p className="text-muted-foreground mt-1 text-xs">
|
||||
{t(
|
||||
'This installation builds its own images, so pulling one changes nothing. If the release moved composer.lock or the frontend, run composer install and npm run build as well — UPDATE.md has the full procedure.',
|
||||
)}
|
||||
</p>
|
||||
</>
|
||||
);
|
||||
}
|
||||
|
||||
if (compact) {
|
||||
return (
|
||||
<div className="text-muted-foreground space-y-1">
|
||||
|
||||
@@ -11,19 +11,30 @@ use Inertia\Testing\AssertableInertia;
|
||||
|
||||
/**
|
||||
* The application tells administrators how to upgrade, and the answer is
|
||||
* different for a container and for files unpacked onto a server. Getting it
|
||||
* wrong is worse than saying nothing: before this existed, both surfaces
|
||||
* printed `docker compose pull` to everybody, including the people INSTALL.md
|
||||
* was written for, who have no docker to run it with.
|
||||
* different for the published image, for a container somebody builds from a
|
||||
* checkout, and for files unpacked onto a server. Getting it wrong is worse
|
||||
* than saying nothing: before this existed, every surface printed
|
||||
* `docker compose pull` to everybody, including the people INSTALL.md was
|
||||
* written for, who have no docker to run it with — and including the
|
||||
* clone-and-build stacks where those commands succeed without updating
|
||||
* anything (#1661).
|
||||
*/
|
||||
class FakeInstallation extends Installation
|
||||
{
|
||||
public function __construct(private readonly bool $container) {}
|
||||
public function __construct(
|
||||
private readonly bool $container,
|
||||
private readonly bool $source = false,
|
||||
) {}
|
||||
|
||||
protected function inContainer(): bool
|
||||
{
|
||||
return $this->container;
|
||||
}
|
||||
|
||||
protected function builtFromSource(): bool
|
||||
{
|
||||
return $this->source;
|
||||
}
|
||||
}
|
||||
|
||||
beforeEach(function () {
|
||||
@@ -44,37 +55,74 @@ function anUpdateIsAvailable(): void
|
||||
$settings->set(Setting::LatestReleasePublishedAt, '2026-08-09T00:00:00Z');
|
||||
}
|
||||
|
||||
test('a container reports itself as one', function () {
|
||||
test('a container running the published image reports itself as one', function () {
|
||||
expect((new FakeInstallation(container: true))->kind())->toBe(InstallationKind::Container);
|
||||
});
|
||||
|
||||
test('a container built from a checkout is told apart from the image', function () {
|
||||
// `docker compose pull` on this stack skips every ProjectSend service
|
||||
// and then reports success, so being handed that command is how an
|
||||
// installation stays on the version it is on.
|
||||
expect((new FakeInstallation(container: true, source: true))->kind())->toBe(InstallationKind::ContainerSource);
|
||||
});
|
||||
|
||||
test('a working tree is only asked about inside a container', function () {
|
||||
expect((new FakeInstallation(container: false, source: true))->kind())->toBe(InstallationKind::Manual);
|
||||
});
|
||||
|
||||
test('the image says so itself, whatever is on disk', function () {
|
||||
// Precedence, asserted where it matters: the test suite runs from a
|
||||
// working tree, so without the marker this same object answers
|
||||
// ContainerSource. An operator bind-mounting a checkout into the
|
||||
// published image is still updating it by pulling.
|
||||
$installation = new class extends Installation
|
||||
{
|
||||
protected function inContainer(): bool
|
||||
{
|
||||
return true;
|
||||
}
|
||||
};
|
||||
|
||||
expect($installation->kind())->toBe(InstallationKind::ContainerSource);
|
||||
|
||||
putenv('PROJECTSEND_IMAGE=1');
|
||||
|
||||
try {
|
||||
expect($installation->kind())->toBe(InstallationKind::Container);
|
||||
} finally {
|
||||
putenv('PROJECTSEND_IMAGE');
|
||||
}
|
||||
})->skip(fn () => ! file_exists(base_path('.git')), 'Asserts precedence over a working tree, and there is none here.');
|
||||
|
||||
test('anything else is treated as a manual install', function () {
|
||||
// The safer wrong answer: manual instructions are steps a person reads
|
||||
// and checks, the container command is one they would paste.
|
||||
expect((new FakeInstallation(container: false))->kind())->toBe(InstallationKind::Manual);
|
||||
});
|
||||
|
||||
test('the dashboard tells the frontend which kind of install this is', function (bool $container, string $expected) {
|
||||
app()->instance(Installation::class, new FakeInstallation($container));
|
||||
test('the dashboard tells the frontend which kind of install this is', function (bool $container, bool $source, string $expected) {
|
||||
app()->instance(Installation::class, new FakeInstallation($container, $source));
|
||||
|
||||
$this->actingAs($this->admin)
|
||||
->get('/dashboard')
|
||||
->assertInertia(fn (AssertableInertia $page) => $page->where('system.install_kind', $expected));
|
||||
})->with([
|
||||
'container' => [true, 'container'],
|
||||
'manual' => [false, 'manual'],
|
||||
'the published image' => [true, false, 'container'],
|
||||
'built from a checkout' => [true, true, 'container-source'],
|
||||
'manual' => [false, false, 'manual'],
|
||||
]);
|
||||
|
||||
test('the update notice carries the install kind too', function (bool $container, string $expected) {
|
||||
app()->instance(Installation::class, new FakeInstallation($container));
|
||||
test('the update notice carries the install kind too', function (bool $container, bool $source, string $expected) {
|
||||
app()->instance(Installation::class, new FakeInstallation($container, $source));
|
||||
anUpdateIsAvailable();
|
||||
|
||||
$this->actingAs($this->admin)
|
||||
->get('/dashboard')
|
||||
->assertInertia(fn (AssertableInertia $page) => $page->where('update_notice.install_kind', $expected));
|
||||
})->with([
|
||||
'container' => [true, 'container'],
|
||||
'manual' => [false, 'manual'],
|
||||
'the published image' => [true, false, 'container'],
|
||||
'built from a checkout' => [true, true, 'container-source'],
|
||||
'manual' => [false, false, 'manual'],
|
||||
]);
|
||||
|
||||
test('no update means no notice, whatever the install kind', function () {
|
||||
|
||||
@@ -18,12 +18,20 @@ use App\Modules\Platform\Settings\Settings;
|
||||
*/
|
||||
class NoticeInstallation extends Installation
|
||||
{
|
||||
public function __construct(private readonly bool $container) {}
|
||||
public function __construct(
|
||||
private readonly bool $container,
|
||||
private readonly bool $source = false,
|
||||
) {}
|
||||
|
||||
protected function inContainer(): bool
|
||||
{
|
||||
return $this->container;
|
||||
}
|
||||
|
||||
protected function builtFromSource(): bool
|
||||
{
|
||||
return $this->source;
|
||||
}
|
||||
}
|
||||
|
||||
function applied(string $version): void
|
||||
@@ -107,14 +115,15 @@ test('it is not gated on the edition', function () {
|
||||
expect(noticeFor($this->admin))->not->toBeNull();
|
||||
});
|
||||
|
||||
test('it names the command this kind of installation can actually run', function (bool $container, string $expected) {
|
||||
app()->instance(Installation::class, new NoticeInstallation($container));
|
||||
test('it names the command this kind of installation can actually run', function (bool $container, bool $source, string $expected) {
|
||||
app()->instance(Installation::class, new NoticeInstallation($container, $source));
|
||||
applied('2.2.0');
|
||||
|
||||
expect(noticeFor($this->admin)['install_kind'])->toBe($expected);
|
||||
})->with([
|
||||
'a container' => [true, InstallationKind::Container->value],
|
||||
'a server somebody administers' => [false, InstallationKind::Manual->value],
|
||||
'a container from the published image' => [true, false, InstallationKind::Container->value],
|
||||
'a container built from a checkout' => [true, true, InstallationKind::ContainerSource->value],
|
||||
'a server somebody administers' => [false, false, InstallationKind::Manual->value],
|
||||
]);
|
||||
|
||||
// The rollback story, asserted end to end: whatever the marker said, the
|
||||
|
||||
Reference in New Issue
Block a user