New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 714165 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

LUCI "system/environ" package is not case-insensitive.

Project Member Reported by d...@chromium.org, Apr 21 2017

Issue description

The LUCI "system/environ" package is not case-insensitive, regarding "FOO" and "foo" as different environment variables. I discovered this when playing around with the Git wrapper and noticing that it couldn't find Git on my test machine's "Path".

We'll probably need to redeploy any packages that depend on this once the patch lands. Comes to mind:
- Git wrapper
- Kitchen
- LogDog Butler / Annotee
- Anything using "lucictx"

ATM things are working, and have been in general, so it's apparently not disastrous. It's been this way for over a year. This is a pretty dumb bug, though ... there's really no excuse. The unit tests were even OK with this...
 

Comment 1 by d...@chromium.org, Apr 21 2017

vadimsh@ pointed out that only Windows is case-insensitive, so that's good. Maybe there is an excuse and the bug isn't as dumb as it seems :)
Project Member

Comment 2 by bugdroid1@chromium.org, Apr 21 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/external/github.com/luci/luci-go.git/+/fde97cf33c7fad766b10ad40b5cbc10830d5749f

commit fde97cf33c7fad766b10ad40b5cbc10830d5749f
Author: dnj <dnj@chromium.org>
Date: Fri Apr 21 18:51:55 2017

environ: Fix key case, add GetEmpty and Remove.

The "environ" package does not ignore the case of enviornment variable
keys on Windows. This patch fixes this bug by allowing an Env to be
configured to be case insensitive, and having Windows systems default to
"true".

Creation methods "System" and "New" will both default to the case
sensitivity of the current operating system.

Additionally, this patch adds Remove, which allows for the deletion of
enviornment variables, and GetEmpty, which offers an interface similar
to "os.Getenv".

BUG= chromium:714165 
TEST=unit

Review-Url: https://codereview.chromium.org/2833073004

[modify] https://crrev.com/fde97cf33c7fad766b10ad40b5cbc10830d5749f/common/system/environ/environment.go
[modify] https://crrev.com/fde97cf33c7fad766b10ad40b5cbc10830d5749f/common/system/environ/environment_test.go

Comment 3 by d...@chromium.org, Apr 21 2017

Should be fixed now, still need to roll Windows versions of these things.

Comment 4 by d...@chromium.org, May 22 2017

Status: Fixed (was: Started)

Sign in to add a comment