public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] BaseTools: Add support for dots in paths
@ 2018-10-05 17:13 Matthew Yeazel
  2018-10-05 18:13 ` Carsey, Jaben
  0 siblings, 1 reply; 4+ messages in thread
From: Matthew Yeazel @ 2018-10-05 17:13 UTC (permalink / raw)
  To: edk2-devel; +Cc: Matthew Yeazel

The split assumes that there isn't a dot in the path to the file but
this isn't always the case. This will support more diverse paths.

Contributed-under: TianoCore Contribution Agreement 1.1
    Note: Section 3, paragraph 1, is read as an OR.
Signed-off-by: Matthew Yeazel <yeazelm@amazon.com>
---
 BaseTools/Source/Python/AutoGen/AutoGen.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py b/BaseTools/Source/Python/AutoGen/AutoGen.py
index eb1b283889..54ad4a5247 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -3062,7 +3062,7 @@ class ModuleAutoGen(AutoGen):
         self.BuildOption
         for SingleFile in FileList:
             if self.BuildRuleOrder and SingleFile.Ext in self.BuildRuleOrder and SingleFile.Ext in self.BuildRules:
-                key = SingleFile.Path.split(SingleFile.Ext)[0]
+                key = SingleFile.Path.rsplit(SingleFile.Ext, 1)[0]
                 if key in Order_Dict:
                     Order_Dict[key].append(SingleFile.Ext)
                 else:
-- 
2.19.0



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] BaseTools: Add support for dots in paths
  2018-10-05 17:13 [PATCH] BaseTools: Add support for dots in paths Matthew Yeazel
@ 2018-10-05 18:13 ` Carsey, Jaben
  2018-10-05 20:07   ` Yeazel (Prime Air), Matt
  0 siblings, 1 reply; 4+ messages in thread
From: Carsey, Jaben @ 2018-10-05 18:13 UTC (permalink / raw)
  To: Matthew Yeazel, edk2-devel@lists.01.org

Matthew,

Would os.path.<something> (maybe splitext) be a better function than using the string  rsplit function?  It just seems like we should use the python file path manipulation functions instead of trying to trick the string manipulation routines into the same behavior...

-Jaben

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Matthew Yeazel
> Sent: Friday, October 05, 2018 10:13 AM
> To: edk2-devel@lists.01.org
> Cc: Matthew Yeazel <yeazelm@amazon.com>
> Subject: [edk2] [PATCH] BaseTools: Add support for dots in paths
> 
> The split assumes that there isn't a dot in the path to the file but
> this isn't always the case. This will support more diverse paths.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
>     Note: Section 3, paragraph 1, is read as an OR.
> Signed-off-by: Matthew Yeazel <yeazelm@amazon.com>
> ---
>  BaseTools/Source/Python/AutoGen/AutoGen.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py
> b/BaseTools/Source/Python/AutoGen/AutoGen.py
> index eb1b283889..54ad4a5247 100644
> --- a/BaseTools/Source/Python/AutoGen/AutoGen.py
> +++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
> @@ -3062,7 +3062,7 @@ class ModuleAutoGen(AutoGen):
>          self.BuildOption
>          for SingleFile in FileList:
>              if self.BuildRuleOrder and SingleFile.Ext in self.BuildRuleOrder and
> SingleFile.Ext in self.BuildRules:
> -                key = SingleFile.Path.split(SingleFile.Ext)[0]
> +                key = SingleFile.Path.rsplit(SingleFile.Ext, 1)[0]
>                  if key in Order_Dict:
>                      Order_Dict[key].append(SingleFile.Ext)
>                  else:
> --
> 2.19.0
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] BaseTools: Add support for dots in paths
  2018-10-05 18:13 ` Carsey, Jaben
@ 2018-10-05 20:07   ` Yeazel (Prime Air), Matt
  2018-10-05 20:20     ` Carsey, Jaben
  0 siblings, 1 reply; 4+ messages in thread
From: Yeazel (Prime Air), Matt @ 2018-10-05 20:07 UTC (permalink / raw)
  To: Carsey, Jaben, edk2-devel@lists.01.org

splitext is probably the exact function we want. I noticed os.path is used extensively through this file but not in this case and was concerned there was a reason behind using SingleFile.Ext to split so I chose to change it as minimally as possible to avoid unintended side effects. I can switch to using os.path.splitext as well if that would be more desired.

Matthew
________________________________________
From: Carsey, Jaben <jaben.carsey@intel.com>
Sent: Friday, October 5, 2018 11:13 AM
To: Yeazel (Prime Air), Matt; edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH] BaseTools: Add support for dots in paths

Matthew,

Would os.path.<something> (maybe splitext) be a better function than using the string  rsplit function?  It just seems like we should use the python file path manipulation functions instead of trying to trick the string manipulation routines into the same behavior...

-Jaben

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Matthew Yeazel
> Sent: Friday, October 05, 2018 10:13 AM
> To: edk2-devel@lists.01.org
> Cc: Matthew Yeazel <yeazelm@amazon.com>
> Subject: [edk2] [PATCH] BaseTools: Add support for dots in paths
>
> The split assumes that there isn't a dot in the path to the file but
> this isn't always the case. This will support more diverse paths.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
>     Note: Section 3, paragraph 1, is read as an OR.
> Signed-off-by: Matthew Yeazel <yeazelm@amazon.com>
> ---
>  BaseTools/Source/Python/AutoGen/AutoGen.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py
> b/BaseTools/Source/Python/AutoGen/AutoGen.py
> index eb1b283889..54ad4a5247 100644
> --- a/BaseTools/Source/Python/AutoGen/AutoGen.py
> +++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
> @@ -3062,7 +3062,7 @@ class ModuleAutoGen(AutoGen):
>          self.BuildOption
>          for SingleFile in FileList:
>              if self.BuildRuleOrder and SingleFile.Ext in self.BuildRuleOrder and
> SingleFile.Ext in self.BuildRules:
> -                key = SingleFile.Path.split(SingleFile.Ext)[0]
> +                key = SingleFile.Path.rsplit(SingleFile.Ext, 1)[0]
>                  if key in Order_Dict:
>                      Order_Dict[key].append(SingleFile.Ext)
>                  else:
> --
> 2.19.0
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] BaseTools: Add support for dots in paths
  2018-10-05 20:07   ` Yeazel (Prime Air), Matt
@ 2018-10-05 20:20     ` Carsey, Jaben
  0 siblings, 0 replies; 4+ messages in thread
From: Carsey, Jaben @ 2018-10-05 20:20 UTC (permalink / raw)
  To: Yeazel (Prime Air), Matt, edk2-devel@lists.01.org

I think that would be better since we are manipulating a path.  I think that file needs lots of rework, but I also know that Bob Feng (added to CC) is working on that file recently and he might want to think on using os.path to replace lots of stuff.

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Yeazel (Prime Air), Matt
> Sent: Friday, October 05, 2018 1:08 PM
> To: Carsey, Jaben <jaben.carsey@intel.com>; edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH] BaseTools: Add support for dots in paths
> Importance: High
> 
> splitext is probably the exact function we want. I noticed os.path is used
> extensively through this file but not in this case and was concerned there
> was a reason behind using SingleFile.Ext to split so I chose to change it as
> minimally as possible to avoid unintended side effects. I can switch to using
> os.path.splitext as well if that would be more desired.
> 
> Matthew
> ________________________________________
> From: Carsey, Jaben <jaben.carsey@intel.com>
> Sent: Friday, October 5, 2018 11:13 AM
> To: Yeazel (Prime Air), Matt; edk2-devel@lists.01.org
> Subject: RE: [edk2] [PATCH] BaseTools: Add support for dots in paths
> 
> Matthew,
> 
> Would os.path.<something> (maybe splitext) be a better function than using
> the string  rsplit function?  It just seems like we should use the python file
> path manipulation functions instead of trying to trick the string manipulation
> routines into the same behavior...
> 
> -Jaben
> 
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> > Matthew Yeazel
> > Sent: Friday, October 05, 2018 10:13 AM
> > To: edk2-devel@lists.01.org
> > Cc: Matthew Yeazel <yeazelm@amazon.com>
> > Subject: [edk2] [PATCH] BaseTools: Add support for dots in paths
> >
> > The split assumes that there isn't a dot in the path to the file but
> > this isn't always the case. This will support more diverse paths.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> >     Note: Section 3, paragraph 1, is read as an OR.
> > Signed-off-by: Matthew Yeazel <yeazelm@amazon.com>
> > ---
> >  BaseTools/Source/Python/AutoGen/AutoGen.py | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py
> > b/BaseTools/Source/Python/AutoGen/AutoGen.py
> > index eb1b283889..54ad4a5247 100644
> > --- a/BaseTools/Source/Python/AutoGen/AutoGen.py
> > +++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
> > @@ -3062,7 +3062,7 @@ class ModuleAutoGen(AutoGen):
> >          self.BuildOption
> >          for SingleFile in FileList:
> >              if self.BuildRuleOrder and SingleFile.Ext in self.BuildRuleOrder and
> > SingleFile.Ext in self.BuildRules:
> > -                key = SingleFile.Path.split(SingleFile.Ext)[0]
> > +                key = SingleFile.Path.rsplit(SingleFile.Ext, 1)[0]
> >                  if key in Order_Dict:
> >                      Order_Dict[key].append(SingleFile.Ext)
> >                  else:
> > --
> > 2.19.0
> >
> > _______________________________________________
> > 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


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-10-05 20:24 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-10-05 17:13 [PATCH] BaseTools: Add support for dots in paths Matthew Yeazel
2018-10-05 18:13 ` Carsey, Jaben
2018-10-05 20:07   ` Yeazel (Prime Air), Matt
2018-10-05 20:20     ` Carsey, Jaben

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox