Skip to content

Conversation

@NoahvdAa
Copy link
Member

@NoahvdAa NoahvdAa commented Oct 2, 2021

This PR re-adds the root reverted in e8830b2. The check was throwing false positives because of a OpenJDk bug (no issue # yet, will add here later). This PR uses a different method for getting the UID (executing the id command), which isn't affected by the openjdk issue.

@NoahvdAa NoahvdAa requested review from a team as code owners October 2, 2021 18:47
@clrxbl
Copy link
Member

clrxbl commented Oct 2, 2021

Why run id -u with the username specifically?
The command should already return the current user's ID.

@Prof-Bloodstone
Copy link
Contributor

Wondering if it'd be better to use available API as before, and if it returns root as user, then use the command to confirm it?
This way it could potentially be faster and doesn't require for the id binary to be on the system most of the time.

@me4502
Copy link
Member

me4502 commented Oct 3, 2021

How common is it for servers to not contain the id binary?

I'd question whether it's better to have it default to warning when it's missing - but guess it depends how often it's missing

Logic being a host can easily fix it themselves if it shows up due to missing id binary

@NoahvdAa NoahvdAa requested review from Proximyst and removed request for a team October 5, 2021 14:29
@NoahvdAa NoahvdAa requested a review from lynxplay October 8, 2021 14:07
@me4502 me4502 force-pushed the feature/re-readd-root-detection branch from 0d4e696 to f988ac1 Compare October 9, 2021 09:17
@me4502 me4502 merged commit cd610df into PaperMC:master Oct 9, 2021
@NoahvdAa NoahvdAa deleted the feature/re-readd-root-detection branch October 9, 2021 10:07
jcxldn added a commit to jcxldn/mcr that referenced this pull request Oct 16, 2021
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.

6 participants