Page MenuHomePhorge

Config page: add $HOME to allow a gitconfig and help on "dubious ownership"
AbandonedPublic

Authored by valerio.bozzolan on Apr 29 2023, 23:44.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Mar 24, 03:42
Unknown Object (File)
Wed, Mar 20, 08:02
Unknown Object (File)
Sat, Mar 16, 21:49
Unknown Object (File)
Sat, Mar 16, 21:49
Unknown Object (File)
Sat, Mar 16, 21:49
Unknown Object (File)
Sat, Mar 16, 21:49
Unknown Object (File)
Sat, Mar 16, 21:49
Unknown Object (File)
Sat, Mar 9, 18:30

Details

Summary

If your Config page was showing some "Unknown" fields, this change is useful,
since it allows you to fix your situation from the .gitconfig of the
user running your webserver.

Before this change, some git commands here were executed without any HOME, so,
git was not opening any .gitconfig, so, any repository marked as "safe" was just
not assumed as safe, so, git was quitting with the exit status 128 with the famous
"detected dubious ownership" error, so, Phorge was not able to run "git log" and
identify the current version, so, Phorge was showing an "Unknown" version.

After this change, these git commands now have the HOME.

You could definitely still have that error. But at least, now you can fix.

After this change, in most cases, git will try to open this file:

/var/www/.gitconfig

So if you want to fix the "detected dubious ownership", now that file is considered,
so you can try to fix with this configuration or a similar one:

/var/www/.gitconfig
[safe]
        directory = /var/www/arcanist
        directory = /var/www/phorge

Note that in normal conditions that configuration is generated from a similar git
global command, but you must issue this from your webserver user:

git config --global --add safe.directory /var/www/phorge

The only current workaround was to fix at --system level instead of --global.

Closes T15299
Ref T15282
Ref T15281
Ref T15298

Test Plan

I finally see my Version(s) instead of "Unknown" since I fixed the .gitconfig file
of the www-data user with the indications of the error logs (from D25148).

Diff Detail

Repository
rP Phorge
Branch
arcpatch-D25149_1
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 480
Build 480: arc lint + arc unit

Event Timeline

create a dedicated method for this specific problem, in order to keep the main method shorter

valerio.bozzolan edited the summary of this revision. (Show Details)

Before this change, some git commands here were executed without any HOME...

Can we see this effect directly? As far as I can tell, HOME is always defined to some value...

src/applications/config/controller/PhabricatorConfigConsoleController.php
362

can we avoid returning false here? it's just confusing.

378

return ASAP, don't keep the value for later.

src/applications/config/controller/PhabricatorConfigConsoleController.php
382

Phorge doesn't run on Windows.

valerio.bozzolan marked 3 inline comments as done.

Do not leave any hope to Microsoft Windows users

In D25149#7386, @avivey wrote:

Before this change, some git commands here were executed without any HOME...

Can we see this effect directly? As far as I can tell, HOME is always defined to some value...

How have you obtained an HOME?

I also tried to issue some echo $HOME without anything on my machine (mod_php). Always undefined. Only after this patch I have finally something.

Small clarification:

To do tests, you can just replace all the commands:

$log_command = csprintf(
  'git log --format=%s -n 1 --',
  '%H %ct');

With something debuggy like this:

$log_command = 'echo $HOME';

Without this patch, the HOME is always missing to me.

The config page shows correct versions both here and in my personal install, so I guess they have a $HOME.

I remember somebody mentioned some E configuration?

In D25149#7663, @avivey wrote:

The config page shows correct versions both here and in my personal install, so I guess they have a $HOME.

I remember somebody mentioned some E configuration?

Can I ask your git version?

I think there is a possibility that your commands has not an home, but you have just a nice old git version that does not stress for "safe" stuff

Screenshot of the current config page of this install:

image.png (463×398 px, 19 KB)

I /think/ I remember setting safedirs at some point here. But it's also possible that it's owned by the same user.

Uhmmmmmm. Maybe I need also to test PHP-FPM instead of just mod_php.

avivey requested changes to this revision.Jun 10 2023, 08:47

For now, rejecting this change until we can get a better understanding of the root issue.

This revision now requires changes to proceed.Jun 10 2023, 08:47

From reading the docs, safe.directory should respect command-line arguments, but at least in 2.34.1 it doesn't. If recent versions do respect it, it might be simpler to just add -c safe.directory=.... to the command rather then passing $HOME.

I agree with a more targeted change of a git-specific argument if possible

I understand. Premising that it's probably too much overkill to design a dedicated Phorge configuration, to pass a valid gitconfig, to then be able to... see the Phorge version.

So, at the moment the only feasible solution is to update the documentation to mention the need of a git "system" configuration (config created using root privileges):

D25236: Diviner: mention how to flag Arcanist and Phorge as "safe" git repos

Or something like this