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

Fall back to HOME environment variable #2

Merged
merged 1 commit into from
Mar 5, 2015
Merged

Conversation

dscho
Copy link
Member

@dscho dscho commented Mar 5, 2015

So far, MSys2 always insists on using /home/ as home
directory, even when the HOME variable is set.

For consistency, let's adopt Git for Windows' strategy to look
at the HOME variable, falling back to $HOMEDRIVE$HOMEPAT, and falling
back even further to $USERPROFILE.

Signed-off-by: Johannes Schindelin johannes.schindelin@gmx.de

@dscho
Copy link
Member Author

dscho commented Mar 5, 2015

This fixes the OpenSSH problem here (where ssh looks at /home/<username>/.ssh instead of $HOME/.ssh/).

@sschuberth @t-b could you have a look?

if (!home)
{
char *env = NULL;
env = getenv("HOME");

This comment was marked as off-topic.

This comment was marked as off-topic.

@sschuberth
Copy link

Looks good to me know. Note that I haven't checked that uinfo.cc is actually the right place to add this piece of code, but I believe it is since you've verified that your code works. Feel free to merge unless you also want @t-b to have a look.

@dscho
Copy link
Member Author

dscho commented Mar 5, 2015

Note that I haven't checked that uinfo.cc is actually the right place to add this piece of code, but I believe it is since you've verified that your code works.

Yeah, I should put in a little bit more information into the commit message. Sorry & thanks for pointing it out!

@t-b
Copy link

t-b commented Mar 5, 2015

minor nit: In the commit message it should be HOMEPATH with H.

@dscho dscho force-pushed the home branch 2 times, most recently from 014ef9f to 73c9509 Compare March 5, 2015 15:01
So far, MSys2 always insists on using /home/<username> as home
directory, even when the HOME variable is set.

For consistency, let's adopt Git for Windows' strategy to look
at the HOME variable, falling back to $HOMEDRIVE$HOMEPATH, and falling
back even further to $USERPROFILE.

This is particularly important when OpenSSH looks for the .ssh/
directory: it actually asks getpwuid() to determine the user's
home directory. The msys2-runtime in turn asks the Cygwin's user
info cache:

https://github.com/Alexpux/Cygwin/blob/6e95f9f5/winsup/cygwin/passwd.cc#L114

This user info cache is initialized during user_info::initialize() in
winsup/cygwin/shared.cc which through a chain of calls ends up calling
pwdgrp::fetch_account_from_windows().

Incidentally, the fetch_account_from_windows() function is the only source
identified by a `git grep /home/` which this developer ran to find out
where the erroneous path came from.

It turns out that the fetch_accound_from_windows() function fails to
identify the home directory which is why MSys2's fallback kicks in. By
enhancing that fallback to follow Git for Windows' reasoning, we fix *all*
MSys2 programs to find the correct home directory.

Assisted-by: Sebastian Schuberth <sschuberth@gmail.com>
Assisted-by: Thomas Braun <thomas.braun@byte-physics.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho
Copy link
Member Author

dscho commented Mar 5, 2015

Lets suppose you set the variables HOMEDRIVE, HOMEPATH and USERPROFILE but not HOME.
You malloc memory in line 2450, set env in 2456, and malloc again in 2459 creating a memleak. Is that correct thinking?

D'oh! In an earlier version, I had the USERPROFILE in an else block... Good point! I now check both home and env before falling back to USERPROFILE.

In the commit message it should be HOMEPATH with H.

Fixed, too.

@t-b
Copy link

t-b commented Mar 5, 2015

Looks good!

t-b added a commit that referenced this pull request Mar 5, 2015
Fall back to HOME environment variable
@t-b t-b merged commit 5292c9e into git-for-windows:develop Mar 5, 2015
@dscho dscho deleted the home branch March 5, 2015 15:19
@dscho
Copy link
Member Author

dscho commented Mar 5, 2015

Thank you!

@dscho
Copy link
Member Author

dscho commented Apr 21, 2015

@Alexpux unfortunately, your suggestion caused us trouble: git-for-windows/git#100. How would you suggest to proceed?

@Alexpux
Copy link

Alexpux commented Apr 21, 2015

don't set HOME variable

@dscho
Copy link
Member Author

dscho commented Apr 22, 2015

don't set HOME variable

@Alexpux I am certain that you can do much better than throwing out this single line. It is not even a well-considered statement, given that MSys1 did respect the HOME variable, and in order to maintain a smooth upgrade path, MSys2 must respect it, too.

So let me summarize the problem: Cygwin's technique to determine the home directory when windows is added to the nsswitch.conf file might be technically correct, but is practically unhelpful. That is why MSys1 used the HOME variable instead (which had to be set by the user before the runtime is initialized).

And this is how I intend to solve the problem: come up with a patch that uses the HOME variable, with a fallback to $HOMEDRIVE$HOMEPATH, as I had it before (because it is a tried and tested method), that gets activated by the env keyword in the nsswitch.conf.

This patch will first make it into git-for-windows/msys2-runtime. After that, I will submit it to the MSys2 project. If @Alexpux has reasonable feedback, I will work with him on integrating it. Otherwise there is always the option to go directly to the Cygwin project, no harm inflicted.

@Alexpux
Copy link

Alexpux commented Apr 22, 2015

MSYS2 is not MSYS1 fork. MSYS2 is based on recent Cygwin sources, so it have the same home directory settings as Cygwin in present time.
I think you need carefully read Cygwin doc about it:
https://cygwin.com/cygwin-ug-net/ntsec.html#ntsec-mapping-passwdinfo

What problem with /etc/nsswitch.conf file?

@dscho
Copy link
Member Author

dscho commented Apr 22, 2015

What problem with /etc/nsswitch.conf file?

Sorry, I thought you read this ticket: git-for-windows/git#100

To recapitulate: The problem is that setting the db_home to windows sometimes results in bogus home directories that cannot even be accessed (e.g. //irvfs007.some.internal.domain/user$).

Previously, we were able to override such faulty behavior by asking users with such setups to define the HOME environment variable explicitly. This is by far the most painless way to cope with this problem, therefore I intend to reinstate that solution.

@dscho
Copy link
Member Author

dscho commented Apr 27, 2015

@Alexpux it would be good if you could read over dscho@bc51eea and commented on it -- keeping in mind that we have to have a way to override the db_home: windows cygwin setting in case it results in a bogus directory (as described here.

dscho pushed a commit that referenced this pull request Nov 9, 2018
Add a new "interleave" allocation policy which stripes pages across
domains with a stride or width keeping contiguity within a multi-page
region.

Move the kernel to the dedicated numbered cpuset #2 making it possible
to assign kernel threads and memory policy separately from user.  This
also eliminates the need for the complicated interrupt binding code.

Add a sysctl API for viewing and manipulating domainsets.  Refactor some
of the cpuset_t manipulation code using the generic bitset type so that
it can be used for both.  This probably belongs in a dedicated subr file.

Attempt to improve the include situation.

Reviewed by:	kib
Discussed with:	jhb (cpuset parts)
Tested by:	pho (before review feedback)
Sponsored by:	Netflix, Dell/EMC Isilon
Differential Revision:	https://reviews.freebsd.org/D14839
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants