Laszlo, The leading "_" was required for out of spec, but built in, commands. The spec has no restrictions on environment variables except some have special meaning and may be read only.
I can certainly work on slowing down the process. I have been complaining about that same thing and should have been more aware. I would like to have a community minimum amount of time before commits are done that we all agree to. Something like 1 full day would be nice I think. -Jaben > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > Andrew Fish > Sent: Thursday, October 04, 2018 12:20 PM > To: Laszlo Ersek <ler...@redhat.com> > Cc: Carsey, Jaben <jaben.car...@intel.com>; edk2-devel@lists.01.org > Subject: Re: [edk2] [PATCH] ShellPkg: Create a homefilesystem environment > variable > Importance: High > > > > > On Oct 4, 2018, at 10:07 AM, Laszlo Ersek <ler...@redhat.com> wrote: > > > > On 10/03/18 20:17, Carsey, Jaben wrote: > >> Pushed. > >> c0b1f749ef1304810ed4ea58ded65b7f41d79d3e > > > > Please give other reviewers a bit more time than ~2 hours, to comment on > > the patch. :) > > > > I think I would have suggested an improvement (or a clarification about) > > the commit message. It says: > > > >>> -----Original Message----- > >>> From: Carsey, Jaben > >>> Sent: Wednesday, October 03, 2018 11:15 AM > >>> To: 'jim.dai...@dell.com' <jim.dai...@dell.com>; edk2- > de...@lists.01.org > >>> Subject: RE: [edk2] [PATCH] ShellPkg: Create a homefilesystem > environment > >>> variable > >>> > >>> Reviewed-by: Jaben Carsey <jaben.car...@intel.com> > >>> > >>>> -----Original Message----- > >>>> From: jim.dai...@dell.com [mailto:jim.dai...@dell.com] > >>>> Sent: Wednesday, October 03, 2018 9:02 AM > >>>> To: edk2-devel@lists.01.org > >>>> Cc: Carsey, Jaben <jaben.car...@intel.com>; Ni, Ruiyu > >>> <ruiyu...@intel.com> > >>>> Subject: [edk2] [PATCH] ShellPkg: Create a homefilesystem > environment > >>>> variable > >>>> Importance: High > >>>> > >>>> Create a homefilesystem environment variable whose value is the file > >>>> system on which the executing shell is located. For example: "FS14:". > > > > that the file system in question contains the *shell*. > > > > So my first question would have been, what if the shell is memory mapped > > (from a firmware volume), but the platform doesn't expose firmware > > filesystems (FFSs) as EFI simple file system protocol instances? In that > > case, the "file system on which the executing shell is located" seems > > ill-defined. > > > > Same if the Shell was network booted. > > Thanks, > > Andrew Fish > > >>>> > >>>> This eliminates the need for people to have to try and find the "boot" > >>>> file system in their startup script. After this change they can simply > > > > Note, here the commit message refers to the startup script, not the > > shell itself. > > > >>>> execute %homefilesystem% to set the cwd to the root of the file > system > >>>> where the shell is located. > > > > I think the commit message here misses a "CD" command. > > > >>>> > >>>> A future enhancement could be to add "homefilesystem" to the list of > >>>> predefined, read-only variables listed in the EfiShellSetEnv function of > >>>> file ShellProtocol.c > > > > Is it OK with the UEFI shell spec to define a shell variable called > > "homefilesystem"? I seem to remember that edk2-specific options for > > standard UEFI shell commands generally start with an underscore, to > > avoid clashing with the standard namespace. Does that apply to shell > > variables perhaps? (This is mostly for my own education.) > > > >>>> > >>>> Contributed-under: TianoCore Contribution Agreement 1.1 > >>>> Signed-off-by: Jim Dailey <jim_dai...@dell.com> > >>>> --- > >>>> ShellPkg/Application/Shell/Shell.c | 2 ++ > >>>> 1 file changed, 2 insertions(+) > >>>> > >>>> diff --git a/ShellPkg/Application/Shell/Shell.c > >>>> b/ShellPkg/Application/Shell/Shell.c > >>>> index 3f3bcbb4b0..6185b6ac80 100644 > >>>> --- a/ShellPkg/Application/Shell/Shell.c > >>>> +++ b/ShellPkg/Application/Shell/Shell.c > >>>> @@ -1169,6 +1169,8 @@ LocateStartupScript ( > >>>> *TempSpot = CHAR_NULL; > >>>> } > >>>> > >>>> + InternalEfiShellSetEnv(L"homefilesystem", StartupScriptPath, > TRUE); > >>>> + > > > > Again, this refers to the startup script, not the shell itself. > > > >>>> StartupScriptPath = StrnCatGrow (&StartupScriptPath, &Size, > >>>> ((FILEPATH_DEVICE_PATH *)FileDevicePath)->PathName, 0); > >>>> PathRemoveLastItem (StartupScriptPath); > >>>> StartupScriptPath = StrnCatGrow (&StartupScriptPath, &Size, > >>>> mStartupScript, 0); > >>>> -- > >>>> 2.17.0.windows.1 > >> > >> _______________________________________________ > >> edk2-devel mailing list > >> edk2-devel@lists.01.org <mailto:edk2-devel@lists.01.org> > >> https://lists.01.org/mailman/listinfo/edk2-devel > <https://lists.01.org/mailman/listinfo/edk2-devel> > >> > > > > Thanks > > Laszlo > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.01.org <mailto:edk2-devel@lists.01.org> > > https://lists.01.org/mailman/listinfo/edk2-devel > <https://lists.01.org/mailman/listinfo/edk2-devel> > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
