Skip to content

Do not overwrite users environment variables#115455

Merged
lszomoru merged 1 commit intomicrosoft:mainfrom
Backfighter:patch-1
Mar 21, 2022
Merged

Do not overwrite users environment variables#115455
lszomoru merged 1 commit intomicrosoft:mainfrom
Backfighter:patch-1

Conversation

@Backfighter
Copy link
Contributor

@Backfighter Backfighter commented Jan 30, 2021

These environment variables are currently enforced by vscode and there is no way to overwrite them. Turning this arround allows the user to use a custom GIT_ASKPASS inside vscode (e.g. /usr/bin/ksshaskpass for kde wallet).

This PR fixes #111839

@joaomoreno
Copy link
Member

joaomoreno commented Feb 1, 2021

The current code prevents accidental usage of that environment variable and breaking auth. So I'd rather we put this behind a configuration setting, given it's such a niche scenario. I would also only move the GIT_ASKPASS env around, not the other two.

@joaomoreno joaomoreno assigned eamodio and unassigned joaomoreno Feb 1, 2021
@ghost
Copy link

ghost commented Feb 1, 2021 •
edited by ghost
Loading

CLA assistant check
All CLA requirements met.

@Backfighter
Copy link
Contributor Author

Backfighter commented Feb 1, 2021

@joaomoreno I have adjusted my changes accordingly.

@joaomoreno joaomoreno changed the base branch from master to main February 15, 2021 08:51
@Backfighter
Copy link
Contributor Author

Backfighter commented Mar 7, 2021

@eamodio What is the status of the merge? Is there anything I can do to speed up the process?

@lszomoru lszomoru assigned lszomoru and unassigned eamodio Oct 4, 2021
@MCterra10
Copy link

MCterra10 commented Dec 22, 2021

Can we have this merged? It's incredibly frustrating

MCterra10
MCterra10 reviewed Dec 28, 2021
View reviewed changes
@Backfighter
Copy link
Contributor Author

Backfighter commented Dec 28, 2021

@MCterra10 thanks for spotting the typo. I have rebased my changes onto the new mainline so it can be merged easily.

@lszomoru any chance this can be merged?

@lszomoru lszomoru added this to the January 2022 milestone Jan 5, 2022
@lszomoru lszomoru modified the milestones: January 2022, February 2022 Jan 27, 2022
@MCterra10
Copy link

MCterra10 commented Feb 1, 2022

@Backfighter conflicts?

@Backfighter
Allow disabling integrated askpass
01ffd21 
The GIT_ASKPASS variable is currently enforced by vscode and there is no way to overwrite it.
This commit adds an options to disable the integrated askpass and use
your own.

Fixes microsoft#111839
@lszomoru lszomoru modified the milestones: February 2022, March 2022 Feb 24, 2022
@MCterra10
Copy link

MCterra10 commented Mar 16, 2022 •
edited
Loading

are the maintainers always this slow for such simple changes?
it's not like there's the "they are volunteers" argument, this is literally a paid job to maintain the vscode repo

@lszomoru lszomoru merged commit b8a5e18 into microsoft:main Mar 21, 2022
@lszomoru
Copy link
Member

lszomoru commented Mar 21, 2022

Apologies for the delay on getting this in. This should be in tomorrow's Insiders release and the next Stable release.

@Backfighter Backfighter deleted the patch-1 branch March 21, 2022 21:46
@MCterra10
Copy link

MCterra10 commented Apr 1, 2022

sweeeeet

@github-actions github-actions bot locked and limited conversation to collaborators May 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@joaomoreno joaomoreno joaomoreno approved these changes

+1 more reviewer

@MCterra10 MCterra10 MCterra10 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@lszomoru lszomoru

Labels

None yet

Projects

None yet

Milestone

March 2022

Development

Successfully merging this pull request may close these issues.

GIT_ASKPASS is set unconditionally

5 participants

@Backfighter @joaomoreno @MCterra10 @lszomoru @eamodio