[PATCH] add top-level .gitattributes file, dealing with .depex


Leif Lindholm <leif.lindholm@...>
 

On Thu, Jul 07, 2016 at 05:03:13PM +0200, Laszlo Ersek wrote:
On 07/07/16 16:24, Leif Lindholm wrote:
Git tends to see .depex files as text, causing hideous patches being
generated (and breaking PatchCheck.py).

Add a .gitattributes file instructing git to treat them as binary.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
---
.gitattributes | 1 +
1 file changed, 1 insertion(+)
create mode 100644 .gitattributes

diff --git a/.gitattributes b/.gitattributes
new file mode 100644
index 0000000..2d8a45b
--- /dev/null
+++ b/.gitattributes
@@ -0,0 +1 @@
+*.depex binary
What generates .depex files? I've never seen any.

Also, unless you add .depex files with "git add" to the set of tracked files, no patches / diffs should cover them. What am I missing? :)

... Hm, after

$ find . -iname "*.depex"

I see .depex files in Build/ (which should be ignored altogether), and

./Vlv2TbltDevicePkg/IntelGopDepex/IntelGopDriver.depex

Why does that file exist in the tree? Let me see... git log says nothing relevant (the file dates back to commit 3cbfba02fef9, "Upload BSD-licensed Vlv2TbltDevicePkg and Vlv2DeviceRefCodePkg to").

Grepping the tree for the filename itself leads to:

Vlv2TbltDevicePkg/PlatformPkg.fdf: DXE_DEPEX DXE_DEPEX Optional $(WORKSPACE)/$(PLATFORM_PACKAGE)/IntelGopDepex/IntelGopDriver.depex
Vlv2TbltDevicePkg/PlatformPkgGcc.fdf: DXE_DEPEX DXE_DEPEX Optional $(WORKSPACE)/$(PLATFORM_PACKAGE)/IntelGopDepex/IntelGopDriver.depex

Do these rules exist to override the DEPEX sections of binary-only modules? If so: that's horrible.

Anyway, given that edk2 contains at least one .depex file, and your patch is correct according to <https://git-scm.com/book/en/v2/Customizing-Git-Git-Attributes>:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Thanks!

I had hoped for comments from someone else on cc, since we don't have
any Maintainers.txt entry for the top level directory :)

But if I don't hear anything before Monday, I'll push it then.

Regards,

Leif


Tim Lewis
 

It appears that this file is not actually used. It is only referenced in the [Rule.Common.UEFI_DRIVER.NATIVE_BINARY] rule in PlatformPkg.fdf. A little further research shows that an alternate method was used for the actual GOP binary (see below). A grep of the entire tree shows that no one uses this rule NATIVE_BINARY. So it looks like it can just be cut out.

BTW, the downside of the method used for the binary version of the GOP driver, is that those drivers cannot use PCDs, since the PCD database is created based on references in the .inf. GOP works because it is pure UEFI and (therefore) doesn't use PCDs.

Tim

FILE DRIVER = FF0C8745-3270-4439-B74F-3E45F8C77064 {
SECTION DXE_DEPEX_EXP = {gPlatformGOPPolicyGuid}
SECTION PE32 = Vlv2MiscBinariesPkg/GOP/7.2.1011/RELEASE_VS2008x86/$(DXE_ARCHITECTURE)/IntelGopDriver.efi
SECTION UI = "IntelGopDriver"
}

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Leif Lindholm
Sent: Friday, July 29, 2016 9:45 AM
To: Laszlo Ersek <lersek@redhat.com>
Cc: michael.d.kinney@intel.com; Jordan Justen <jordan.l.justen@intel.com>; edk2-devel@ml01.01.org; Andrew Fish <afish@apple.com>
Subject: Re: [edk2] [PATCH] add top-level .gitattributes file, dealing with .depex

On Thu, Jul 07, 2016 at 05:03:13PM +0200, Laszlo Ersek wrote:
On 07/07/16 16:24, Leif Lindholm wrote:
Git tends to see .depex files as text, causing hideous patches being
generated (and breaking PatchCheck.py).

Add a .gitattributes file instructing git to treat them as binary.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
---
.gitattributes | 1 +
1 file changed, 1 insertion(+)
create mode 100644 .gitattributes

diff --git a/.gitattributes b/.gitattributes new file mode 100644
index 0000000..2d8a45b
--- /dev/null
+++ b/.gitattributes
@@ -0,0 +1 @@
+*.depex binary
What generates .depex files? I've never seen any.

Also, unless you add .depex files with "git add" to the set of tracked
files, no patches / diffs should cover them. What am I missing? :)

... Hm, after

$ find . -iname "*.depex"

I see .depex files in Build/ (which should be ignored altogether), and

./Vlv2TbltDevicePkg/IntelGopDepex/IntelGopDriver.depex

Why does that file exist in the tree? Let me see... git log says nothing relevant (the file dates back to commit 3cbfba02fef9, "Upload BSD-licensed Vlv2TbltDevicePkg and Vlv2DeviceRefCodePkg to").

Grepping the tree for the filename itself leads to:

Vlv2TbltDevicePkg/PlatformPkg.fdf: DXE_DEPEX DXE_DEPEX Optional $(WORKSPACE)/$(PLATFORM_PACKAGE)/IntelGopDepex/IntelGopDriver.depex
Vlv2TbltDevicePkg/PlatformPkgGcc.fdf: DXE_DEPEX DXE_DEPEX Optional $(WORKSPACE)/$(PLATFORM_PACKAGE)/IntelGopDepex/IntelGopDriver.depex

Do these rules exist to override the DEPEX sections of binary-only modules? If so: that's horrible.

Anyway, given that edk2 contains at least one .depex file, and your patch is correct according to <https://git-scm.com/book/en/v2/Customizing-Git-Git-Attributes>:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Thanks!

I had hoped for comments from someone else on cc, since we don't have any Maintainers.txt entry for the top level directory :)

But if I don't hear anything before Monday, I'll push it then.

Regards,

Leif

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Leif Lindholm <leif.lindholm@...>
 

Hi Tim,

Thanks for the warning, and investigation.

Does this mean that you think we should ban the inclusion of .depex
files in EDK2, including future platform trees? (If not, this patch is
still needed for git to work predictably with these files.)

Regards,

Leif

On Fri, Jul 29, 2016 at 05:12:49PM +0000, Tim Lewis wrote:
It appears that this file is not actually used. It is only
referenced in the [Rule.Common.UEFI_DRIVER.NATIVE_BINARY] rule in
PlatformPkg.fdf. A little further research shows that an alternate
method was used for the actual GOP binary (see below). A grep of the
entire tree shows that no one uses this rule NATIVE_BINARY. So it
looks like it can just be cut out.

BTW, the downside of the method used for the binary version of the
GOP driver, is that those drivers cannot use PCDs, since the PCD
database is created based on references in the .inf. GOP works
because it is pure UEFI and (therefore) doesn't use PCDs.

Tim

FILE DRIVER = FF0C8745-3270-4439-B74F-3E45F8C77064 {
SECTION DXE_DEPEX_EXP = {gPlatformGOPPolicyGuid}
SECTION PE32 = Vlv2MiscBinariesPkg/GOP/7.2.1011/RELEASE_VS2008x86/$(DXE_ARCHITECTURE)/IntelGopDriver.efi
SECTION UI = "IntelGopDriver"
}

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Leif Lindholm
Sent: Friday, July 29, 2016 9:45 AM
To: Laszlo Ersek <lersek@redhat.com>
Cc: michael.d.kinney@intel.com; Jordan Justen <jordan.l.justen@intel.com>; edk2-devel@ml01.01.org; Andrew Fish <afish@apple.com>
Subject: Re: [edk2] [PATCH] add top-level .gitattributes file, dealing with .depex

On Thu, Jul 07, 2016 at 05:03:13PM +0200, Laszlo Ersek wrote:
On 07/07/16 16:24, Leif Lindholm wrote:
Git tends to see .depex files as text, causing hideous patches being
generated (and breaking PatchCheck.py).

Add a .gitattributes file instructing git to treat them as binary.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
---
.gitattributes | 1 +
1 file changed, 1 insertion(+)
create mode 100644 .gitattributes

diff --git a/.gitattributes b/.gitattributes new file mode 100644
index 0000000..2d8a45b
--- /dev/null
+++ b/.gitattributes
@@ -0,0 +1 @@
+*.depex binary
What generates .depex files? I've never seen any.

Also, unless you add .depex files with "git add" to the set of tracked
files, no patches / diffs should cover them. What am I missing? :)

... Hm, after

$ find . -iname "*.depex"

I see .depex files in Build/ (which should be ignored altogether), and

./Vlv2TbltDevicePkg/IntelGopDepex/IntelGopDriver.depex

Why does that file exist in the tree? Let me see... git log says nothing relevant (the file dates back to commit 3cbfba02fef9, "Upload BSD-licensed Vlv2TbltDevicePkg and Vlv2DeviceRefCodePkg to").

Grepping the tree for the filename itself leads to:

Vlv2TbltDevicePkg/PlatformPkg.fdf: DXE_DEPEX DXE_DEPEX Optional $(WORKSPACE)/$(PLATFORM_PACKAGE)/IntelGopDepex/IntelGopDriver.depex
Vlv2TbltDevicePkg/PlatformPkgGcc.fdf: DXE_DEPEX DXE_DEPEX Optional $(WORKSPACE)/$(PLATFORM_PACKAGE)/IntelGopDepex/IntelGopDriver.depex

Do these rules exist to override the DEPEX sections of binary-only modules? If so: that's horrible.

Anyway, given that edk2 contains at least one .depex file, and your patch is correct according to <https://git-scm.com/book/en/v2/Customizing-Git-Git-Attributes>:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Thanks!

I had hoped for comments from someone else on cc, since we don't have any Maintainers.txt entry for the top level directory :)

But if I don't hear anything before Monday, I'll push it then.

Regards,

Leif

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Jordan Justen
 

On 2016-07-30 11:33:43, Leif Lindholm wrote:
Hi Tim,

Thanks for the warning, and investigation.

Does this mean that you think we should ban the inclusion of .depex
files in EDK2, including future platform trees?
I don't know about banning it, but at least we could wait for someone
to make a reasonable argument why they are needed.

Even for binary only modules, it looks like the fdf method outlined
below is preferable to a pre-built .depex.

If (at a future point) the reason for using a .depex is to support a
binary only module in a supposedly open platform under EDK II, then I
guess we can decide if that is a good idea at that point.

Should we delete this one unused .depex from the tree?

-Jordan

(If not, this patch is
still needed for git to work predictably with these files.)

Regards,

Leif

On Fri, Jul 29, 2016 at 05:12:49PM +0000, Tim Lewis wrote:
It appears that this file is not actually used. It is only
referenced in the [Rule.Common.UEFI_DRIVER.NATIVE_BINARY] rule in
PlatformPkg.fdf. A little further research shows that an alternate
method was used for the actual GOP binary (see below). A grep of the
entire tree shows that no one uses this rule NATIVE_BINARY. So it
looks like it can just be cut out.

BTW, the downside of the method used for the binary version of the
GOP driver, is that those drivers cannot use PCDs, since the PCD
database is created based on references in the .inf. GOP works
because it is pure UEFI and (therefore) doesn't use PCDs.

Tim

FILE DRIVER = FF0C8745-3270-4439-B74F-3E45F8C77064 {
SECTION DXE_DEPEX_EXP = {gPlatformGOPPolicyGuid}
SECTION PE32 = Vlv2MiscBinariesPkg/GOP/7.2.1011/RELEASE_VS2008x86/$(DXE_ARCHITECTURE)/IntelGopDriver.efi
SECTION UI = "IntelGopDriver"
}

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Leif Lindholm
Sent: Friday, July 29, 2016 9:45 AM
To: Laszlo Ersek <lersek@redhat.com>
Cc: michael.d.kinney@intel.com; Jordan Justen <jordan.l.justen@intel.com>; edk2-devel@ml01.01.org; Andrew Fish <afish@apple.com>
Subject: Re: [edk2] [PATCH] add top-level .gitattributes file, dealing with .depex

On Thu, Jul 07, 2016 at 05:03:13PM +0200, Laszlo Ersek wrote:
On 07/07/16 16:24, Leif Lindholm wrote:
Git tends to see .depex files as text, causing hideous patches being
generated (and breaking PatchCheck.py).

Add a .gitattributes file instructing git to treat them as binary.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
---
.gitattributes | 1 +
1 file changed, 1 insertion(+)
create mode 100644 .gitattributes

diff --git a/.gitattributes b/.gitattributes new file mode 100644
index 0000000..2d8a45b
--- /dev/null
+++ b/.gitattributes
@@ -0,0 +1 @@
+*.depex binary
What generates .depex files? I've never seen any.

Also, unless you add .depex files with "git add" to the set of tracked
files, no patches / diffs should cover them. What am I missing? :)

... Hm, after

$ find . -iname "*.depex"

I see .depex files in Build/ (which should be ignored altogether), and

./Vlv2TbltDevicePkg/IntelGopDepex/IntelGopDriver.depex

Why does that file exist in the tree? Let me see... git log says nothing relevant (the file dates back to commit 3cbfba02fef9, "Upload BSD-licensed Vlv2TbltDevicePkg and Vlv2DeviceRefCodePkg to").

Grepping the tree for the filename itself leads to:

Vlv2TbltDevicePkg/PlatformPkg.fdf: DXE_DEPEX DXE_DEPEX Optional $(WORKSPACE)/$(PLATFORM_PACKAGE)/IntelGopDepex/IntelGopDriver.depex
Vlv2TbltDevicePkg/PlatformPkgGcc.fdf: DXE_DEPEX DXE_DEPEX Optional $(WORKSPACE)/$(PLATFORM_PACKAGE)/IntelGopDepex/IntelGopDriver.depex

Do these rules exist to override the DEPEX sections of binary-only modules? If so: that's horrible.

Anyway, given that edk2 contains at least one .depex file, and your patch is correct according to <https://git-scm.com/book/en/v2/Customizing-Git-Git-Attributes>:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Thanks!

I had hoped for comments from someone else on cc, since we don't have any Maintainers.txt entry for the top level directory :)

But if I don't hear anything before Monday, I'll push it then.

Regards,

Leif

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Michael D Kinney
 

Jordan,

UEFI Drivers distributed as binaries do not need depex sections.

PI modules distributed as binaries do require a .depex binary.

So I would recommend .depex binary files be treated the same as
binary .efi files by GIT. So it does sound like we need some
minor updates to GIT attributes.

If we have an example of a binary module that is providing more
binary leaf sections than are actually required and/or used, then
yes, the binary module should be cleaned up to remove the unused
content.

Thanks,

Mike

-----Original Message-----
From: Justen, Jordan L
Sent: Sunday, July 31, 2016 3:58 PM
To: Leif Lindholm <leif.lindholm@linaro.org>; Tim Lewis <tim.lewis@insyde.com>
Cc: Laszlo Ersek <lersek@redhat.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
edk2-devel@ml01.01.org <edk2-devel@lists.01.org>; Andrew Fish <afish@apple.com>
Subject: Re: [edk2] [PATCH] add top-level .gitattributes file, dealing with .depex

On 2016-07-30 11:33:43, Leif Lindholm wrote:
Hi Tim,

Thanks for the warning, and investigation.

Does this mean that you think we should ban the inclusion of .depex
files in EDK2, including future platform trees?
I don't know about banning it, but at least we could wait for someone
to make a reasonable argument why they are needed.

Even for binary only modules, it looks like the fdf method outlined
below is preferable to a pre-built .depex.

If (at a future point) the reason for using a .depex is to support a
binary only module in a supposedly open platform under EDK II, then I
guess we can decide if that is a good idea at that point.

Should we delete this one unused .depex from the tree?

-Jordan

(If not, this patch is
still needed for git to work predictably with these files.)

Regards,

Leif

On Fri, Jul 29, 2016 at 05:12:49PM +0000, Tim Lewis wrote:
It appears that this file is not actually used. It is only
referenced in the [Rule.Common.UEFI_DRIVER.NATIVE_BINARY] rule in
PlatformPkg.fdf. A little further research shows that an alternate
method was used for the actual GOP binary (see below). A grep of the
entire tree shows that no one uses this rule NATIVE_BINARY. So it
looks like it can just be cut out.

BTW, the downside of the method used for the binary version of the
GOP driver, is that those drivers cannot use PCDs, since the PCD
database is created based on references in the .inf. GOP works
because it is pure UEFI and (therefore) doesn't use PCDs.

Tim

FILE DRIVER = FF0C8745-3270-4439-B74F-3E45F8C77064 {
SECTION DXE_DEPEX_EXP = {gPlatformGOPPolicyGuid}
SECTION PE32 =
Vlv2MiscBinariesPkg/GOP/7.2.1011/RELEASE_VS2008x86/$(DXE_ARCHITECTURE)/IntelGopDriver.e
fi
SECTION UI = "IntelGopDriver"
}

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Leif
Lindholm
Sent: Friday, July 29, 2016 9:45 AM
To: Laszlo Ersek <lersek@redhat.com>
Cc: michael.d.kinney@intel.com; Jordan Justen <jordan.l.justen@intel.com>; edk2-
devel@ml01.01.org; Andrew Fish <afish@apple.com>
Subject: Re: [edk2] [PATCH] add top-level .gitattributes file, dealing with .depex

On Thu, Jul 07, 2016 at 05:03:13PM +0200, Laszlo Ersek wrote:
On 07/07/16 16:24, Leif Lindholm wrote:
Git tends to see .depex files as text, causing hideous patches being
generated (and breaking PatchCheck.py).

Add a .gitattributes file instructing git to treat them as binary.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
---
.gitattributes | 1 +
1 file changed, 1 insertion(+)
create mode 100644 .gitattributes

diff --git a/.gitattributes b/.gitattributes new file mode 100644
index 0000000..2d8a45b
--- /dev/null
+++ b/.gitattributes
@@ -0,0 +1 @@
+*.depex binary
What generates .depex files? I've never seen any.

Also, unless you add .depex files with "git add" to the set of tracked
files, no patches / diffs should cover them. What am I missing? :)

... Hm, after

$ find . -iname "*.depex"

I see .depex files in Build/ (which should be ignored altogether), and

./Vlv2TbltDevicePkg/IntelGopDepex/IntelGopDriver.depex

Why does that file exist in the tree? Let me see... git log says nothing relevant
(the file dates back to commit 3cbfba02fef9, "Upload BSD-licensed Vlv2TbltDevicePkg and
Vlv2DeviceRefCodePkg to").

Grepping the tree for the filename itself leads to:

Vlv2TbltDevicePkg/PlatformPkg.fdf: DXE_DEPEX DXE_DEPEX Optional
$(WORKSPACE)/$(PLATFORM_PACKAGE)/IntelGopDepex/IntelGopDriver.depex
Vlv2TbltDevicePkg/PlatformPkgGcc.fdf: DXE_DEPEX DXE_DEPEX Optional
$(WORKSPACE)/$(PLATFORM_PACKAGE)/IntelGopDepex/IntelGopDriver.depex

Do these rules exist to override the DEPEX sections of binary-only modules? If
so: that's horrible.

Anyway, given that edk2 contains at least one .depex file, and your patch is
correct according to <https://git-scm.com/book/en/v2/Customizing-Git-Git-Attributes>:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Thanks!

I had hoped for comments from someone else on cc, since we don't have any
Maintainers.txt entry for the top level directory :)

But if I don't hear anything before Monday, I'll push it then.

Regards,

Leif

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Jordan Justen
 

On 2016-07-31 16:52:23, Kinney, Michael D wrote:
Jordan,

UEFI Drivers distributed as binaries do not need depex sections.

PI modules distributed as binaries do require a .depex binary.
They may require a depex, but, as mentioned below, they can also add
it directly in the .fdf. As it stands, apparently we have 1 .depex
file in the tree, and it is unused.

Aside from this, under what conditions would we take such binaries
into the EDK II tree? Today we have the ShellPkg and FatPkg binaries
in the EDK II tree, but we recently discussed removing even those.

For an open source project, I think it is best to not have pre-built
binaries, unless there is some very compelling reason. Previously
there was some license funniness on FatPkg, but now that is gone. If
it took an hour to build FatPkg, then that might also be something to
discuss. :)

I don't think adding the .gitattributes is really a problem, aside
from the fact that it implies that we might actually have a reason to
add a .depex file to the source tree.

-Jordan

So I would recommend .depex binary files be treated the same as
binary .efi files by GIT. So it does sound like we need some
minor updates to GIT attributes.

If we have an example of a binary module that is providing more
binary leaf sections than are actually required and/or used, then
yes, the binary module should be cleaned up to remove the unused
content.

Thanks,

Mike

-----Original Message-----
From: Justen, Jordan L
Sent: Sunday, July 31, 2016 3:58 PM
To: Leif Lindholm <leif.lindholm@linaro.org>; Tim Lewis <tim.lewis@insyde.com>
Cc: Laszlo Ersek <lersek@redhat.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
edk2-devel@ml01.01.org <edk2-devel@lists.01.org>; Andrew Fish <afish@apple.com>
Subject: Re: [edk2] [PATCH] add top-level .gitattributes file, dealing with .depex

On 2016-07-30 11:33:43, Leif Lindholm wrote:
Hi Tim,

Thanks for the warning, and investigation.

Does this mean that you think we should ban the inclusion of .depex
files in EDK2, including future platform trees?
I don't know about banning it, but at least we could wait for someone
to make a reasonable argument why they are needed.

Even for binary only modules, it looks like the fdf method outlined
below is preferable to a pre-built .depex.

If (at a future point) the reason for using a .depex is to support a
binary only module in a supposedly open platform under EDK II, then I
guess we can decide if that is a good idea at that point.

Should we delete this one unused .depex from the tree?

-Jordan

(If not, this patch is
still needed for git to work predictably with these files.)

Regards,

Leif

On Fri, Jul 29, 2016 at 05:12:49PM +0000, Tim Lewis wrote:
It appears that this file is not actually used. It is only
referenced in the [Rule.Common.UEFI_DRIVER.NATIVE_BINARY] rule in
PlatformPkg.fdf. A little further research shows that an alternate
method was used for the actual GOP binary (see below). A grep of the
entire tree shows that no one uses this rule NATIVE_BINARY. So it
looks like it can just be cut out.

BTW, the downside of the method used for the binary version of the
GOP driver, is that those drivers cannot use PCDs, since the PCD
database is created based on references in the .inf. GOP works
because it is pure UEFI and (therefore) doesn't use PCDs.

Tim

FILE DRIVER = FF0C8745-3270-4439-B74F-3E45F8C77064 {
SECTION DXE_DEPEX_EXP = {gPlatformGOPPolicyGuid}
SECTION PE32 =
Vlv2MiscBinariesPkg/GOP/7.2.1011/RELEASE_VS2008x86/$(DXE_ARCHITECTURE)/IntelGopDriver.e
fi
SECTION UI = "IntelGopDriver"
}

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Leif
Lindholm
Sent: Friday, July 29, 2016 9:45 AM
To: Laszlo Ersek <lersek@redhat.com>
Cc: michael.d.kinney@intel.com; Jordan Justen <jordan.l.justen@intel.com>; edk2-
devel@ml01.01.org; Andrew Fish <afish@apple.com>
Subject: Re: [edk2] [PATCH] add top-level .gitattributes file, dealing with .depex

On Thu, Jul 07, 2016 at 05:03:13PM +0200, Laszlo Ersek wrote:
On 07/07/16 16:24, Leif Lindholm wrote:
Git tends to see .depex files as text, causing hideous patches being
generated (and breaking PatchCheck.py).

Add a .gitattributes file instructing git to treat them as binary.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
---
.gitattributes | 1 +
1 file changed, 1 insertion(+)
create mode 100644 .gitattributes

diff --git a/.gitattributes b/.gitattributes new file mode 100644
index 0000000..2d8a45b
--- /dev/null
+++ b/.gitattributes
@@ -0,0 +1 @@
+*.depex binary
What generates .depex files? I've never seen any.

Also, unless you add .depex files with "git add" to the set of tracked
files, no patches / diffs should cover them. What am I missing? :)

... Hm, after

$ find . -iname "*.depex"

I see .depex files in Build/ (which should be ignored altogether), and

./Vlv2TbltDevicePkg/IntelGopDepex/IntelGopDriver.depex

Why does that file exist in the tree? Let me see... git log says nothing relevant
(the file dates back to commit 3cbfba02fef9, "Upload BSD-licensed Vlv2TbltDevicePkg and
Vlv2DeviceRefCodePkg to").

Grepping the tree for the filename itself leads to:

Vlv2TbltDevicePkg/PlatformPkg.fdf: DXE_DEPEX DXE_DEPEX Optional
$(WORKSPACE)/$(PLATFORM_PACKAGE)/IntelGopDepex/IntelGopDriver.depex
Vlv2TbltDevicePkg/PlatformPkgGcc.fdf: DXE_DEPEX DXE_DEPEX Optional
$(WORKSPACE)/$(PLATFORM_PACKAGE)/IntelGopDepex/IntelGopDriver.depex

Do these rules exist to override the DEPEX sections of binary-only modules? If
so: that's horrible.

Anyway, given that edk2 contains at least one .depex file, and your patch is
correct according to <https://git-scm.com/book/en/v2/Customizing-Git-Git-Attributes>:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Thanks!

I had hoped for comments from someone else on cc, since we don't have any
Maintainers.txt entry for the top level directory :)

But if I don't hear anything before Monday, I'll push it then.

Regards,

Leif

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Leif Lindholm <leif.lindholm@...>
 

On Mon, Aug 01, 2016 at 12:03:06AM -0700, Jordan Justen wrote:
On 2016-07-31 16:52:23, Kinney, Michael D wrote:
Jordan,

UEFI Drivers distributed as binaries do not need depex sections.

PI modules distributed as binaries do require a .depex binary.
They may require a depex, but, as mentioned below, they can also add
it directly in the .fdf. As it stands, apparently we have 1 .depex
file in the tree, and it is unused.

Aside from this, under what conditions would we take such binaries
into the EDK II tree? Today we have the ShellPkg and FatPkg binaries
in the EDK II tree, but we recently discussed removing even those.
While I don't disagree, the PI dependency expression instruction set
(section 10.7, PI spec 1.4 vol2) does not look Turing complete to me.
Meaning it's "binary" in much the same way a .uni file is.

(This is historically where someone pulls out an operating system
kernel written entirely in PI depex binary.)

For an open source project, I think it is best to not have pre-built
binaries, unless there is some very compelling reason. Previously
there was some license funniness on FatPkg, but now that is gone. If
it took an hour to build FatPkg, then that might also be something to
discuss. :)

I don't think adding the .gitattributes is really a problem, aside
from the fact that it implies that we might actually have a reason to
add a .depex file to the source tree.
And I agree it would send that signal.

Regards,

Leif

-Jordan

So I would recommend .depex binary files be treated the same as
binary .efi files by GIT. So it does sound like we need some
minor updates to GIT attributes.

If we have an example of a binary module that is providing more
binary leaf sections than are actually required and/or used, then
yes, the binary module should be cleaned up to remove the unused
content.

Thanks,

Mike

-----Original Message-----
From: Justen, Jordan L
Sent: Sunday, July 31, 2016 3:58 PM
To: Leif Lindholm <leif.lindholm@linaro.org>; Tim Lewis <tim.lewis@insyde.com>
Cc: Laszlo Ersek <lersek@redhat.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
edk2-devel@ml01.01.org <edk2-devel@lists.01.org>; Andrew Fish <afish@apple.com>
Subject: Re: [edk2] [PATCH] add top-level .gitattributes file, dealing with .depex

On 2016-07-30 11:33:43, Leif Lindholm wrote:
Hi Tim,

Thanks for the warning, and investigation.

Does this mean that you think we should ban the inclusion of .depex
files in EDK2, including future platform trees?
I don't know about banning it, but at least we could wait for someone
to make a reasonable argument why they are needed.

Even for binary only modules, it looks like the fdf method outlined
below is preferable to a pre-built .depex.

If (at a future point) the reason for using a .depex is to support a
binary only module in a supposedly open platform under EDK II, then I
guess we can decide if that is a good idea at that point.

Should we delete this one unused .depex from the tree?

-Jordan

(If not, this patch is
still needed for git to work predictably with these files.)

Regards,

Leif

On Fri, Jul 29, 2016 at 05:12:49PM +0000, Tim Lewis wrote:
It appears that this file is not actually used. It is only
referenced in the [Rule.Common.UEFI_DRIVER.NATIVE_BINARY] rule in
PlatformPkg.fdf. A little further research shows that an alternate
method was used for the actual GOP binary (see below). A grep of the
entire tree shows that no one uses this rule NATIVE_BINARY. So it
looks like it can just be cut out.

BTW, the downside of the method used for the binary version of the
GOP driver, is that those drivers cannot use PCDs, since the PCD
database is created based on references in the .inf. GOP works
because it is pure UEFI and (therefore) doesn't use PCDs.

Tim

FILE DRIVER = FF0C8745-3270-4439-B74F-3E45F8C77064 {
SECTION DXE_DEPEX_EXP = {gPlatformGOPPolicyGuid}
SECTION PE32 =
Vlv2MiscBinariesPkg/GOP/7.2.1011/RELEASE_VS2008x86/$(DXE_ARCHITECTURE)/IntelGopDriver.e
fi
SECTION UI = "IntelGopDriver"
}

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Leif
Lindholm
Sent: Friday, July 29, 2016 9:45 AM
To: Laszlo Ersek <lersek@redhat.com>
Cc: michael.d.kinney@intel.com; Jordan Justen <jordan.l.justen@intel.com>; edk2-
devel@ml01.01.org; Andrew Fish <afish@apple.com>
Subject: Re: [edk2] [PATCH] add top-level .gitattributes file, dealing with .depex

On Thu, Jul 07, 2016 at 05:03:13PM +0200, Laszlo Ersek wrote:
On 07/07/16 16:24, Leif Lindholm wrote:
Git tends to see .depex files as text, causing hideous patches being
generated (and breaking PatchCheck.py).

Add a .gitattributes file instructing git to treat them as binary.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
---
.gitattributes | 1 +
1 file changed, 1 insertion(+)
create mode 100644 .gitattributes

diff --git a/.gitattributes b/.gitattributes new file mode 100644
index 0000000..2d8a45b
--- /dev/null
+++ b/.gitattributes
@@ -0,0 +1 @@
+*.depex binary
What generates .depex files? I've never seen any.

Also, unless you add .depex files with "git add" to the set of tracked
files, no patches / diffs should cover them. What am I missing? :)

... Hm, after

$ find . -iname "*.depex"

I see .depex files in Build/ (which should be ignored altogether), and

./Vlv2TbltDevicePkg/IntelGopDepex/IntelGopDriver.depex

Why does that file exist in the tree? Let me see... git log says nothing relevant
(the file dates back to commit 3cbfba02fef9, "Upload BSD-licensed Vlv2TbltDevicePkg and
Vlv2DeviceRefCodePkg to").

Grepping the tree for the filename itself leads to:

Vlv2TbltDevicePkg/PlatformPkg.fdf: DXE_DEPEX DXE_DEPEX Optional
$(WORKSPACE)/$(PLATFORM_PACKAGE)/IntelGopDepex/IntelGopDriver.depex
Vlv2TbltDevicePkg/PlatformPkgGcc.fdf: DXE_DEPEX DXE_DEPEX Optional
$(WORKSPACE)/$(PLATFORM_PACKAGE)/IntelGopDepex/IntelGopDriver.depex

Do these rules exist to override the DEPEX sections of binary-only modules? If
so: that's horrible.

Anyway, given that edk2 contains at least one .depex file, and your patch is
correct according to <https://git-scm.com/book/en/v2/Customizing-Git-Git-Attributes>:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Thanks!

I had hoped for comments from someone else on cc, since we don't have any
Maintainers.txt entry for the top level directory :)

But if I don't hear anything before Monday, I'll push it then.

Regards,

Leif

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Michael D Kinney
 

Jordan,

I agree that we should not be adding .efi binaries or .depex binary files
to our source repos. There are several repos for EDK II, and some of
those do accept binaries. Specifically, the edk2-non-osi and edk2-platforms
would be repos where .depex may occur. Possibly edk2-staging as well.
I think it is easier to manage things like GIT attributes so they are the
same for all repos in the project.

Mike

-----Original Message-----
From: Leif Lindholm [mailto:leif.lindholm@linaro.org]
Sent: Monday, August 1, 2016 1:19 AM
To: Justen, Jordan L <jordan.l.justen@intel.com>
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Tim Lewis <tim.lewis@insyde.com>;
Laszlo Ersek <lersek@redhat.com>; edk2-devel@lists.01.org; Andrew Fish
<afish@apple.com>
Subject: Re: [edk2] [PATCH] add top-level .gitattributes file, dealing with .depex

On Mon, Aug 01, 2016 at 12:03:06AM -0700, Jordan Justen wrote:
On 2016-07-31 16:52:23, Kinney, Michael D wrote:
Jordan,

UEFI Drivers distributed as binaries do not need depex sections.

PI modules distributed as binaries do require a .depex binary.
They may require a depex, but, as mentioned below, they can also add
it directly in the .fdf. As it stands, apparently we have 1 .depex
file in the tree, and it is unused.

Aside from this, under what conditions would we take such binaries
into the EDK II tree? Today we have the ShellPkg and FatPkg binaries
in the EDK II tree, but we recently discussed removing even those.
While I don't disagree, the PI dependency expression instruction set
(section 10.7, PI spec 1.4 vol2) does not look Turing complete to me.
Meaning it's "binary" in much the same way a .uni file is.

(This is historically where someone pulls out an operating system
kernel written entirely in PI depex binary.)

For an open source project, I think it is best to not have pre-built
binaries, unless there is some very compelling reason. Previously
there was some license funniness on FatPkg, but now that is gone. If
it took an hour to build FatPkg, then that might also be something to
discuss. :)

I don't think adding the .gitattributes is really a problem, aside
from the fact that it implies that we might actually have a reason to
add a .depex file to the source tree.
And I agree it would send that signal.

Regards,

Leif

-Jordan

So I would recommend .depex binary files be treated the same as
binary .efi files by GIT. So it does sound like we need some
minor updates to GIT attributes.

If we have an example of a binary module that is providing more
binary leaf sections than are actually required and/or used, then
yes, the binary module should be cleaned up to remove the unused
content.

Thanks,

Mike

-----Original Message-----
From: Justen, Jordan L
Sent: Sunday, July 31, 2016 3:58 PM
To: Leif Lindholm <leif.lindholm@linaro.org>; Tim Lewis <tim.lewis@insyde.com>
Cc: Laszlo Ersek <lersek@redhat.com>; Kinney, Michael D
<michael.d.kinney@intel.com>;
edk2-devel@ml01.01.org <edk2-devel@lists.01.org>; Andrew Fish <afish@apple.com>
Subject: Re: [edk2] [PATCH] add top-level .gitattributes file, dealing with
.depex

On 2016-07-30 11:33:43, Leif Lindholm wrote:
Hi Tim,

Thanks for the warning, and investigation.

Does this mean that you think we should ban the inclusion of .depex
files in EDK2, including future platform trees?
I don't know about banning it, but at least we could wait for someone
to make a reasonable argument why they are needed.

Even for binary only modules, it looks like the fdf method outlined
below is preferable to a pre-built .depex.

If (at a future point) the reason for using a .depex is to support a
binary only module in a supposedly open platform under EDK II, then I
guess we can decide if that is a good idea at that point.

Should we delete this one unused .depex from the tree?

-Jordan

(If not, this patch is
still needed for git to work predictably with these files.)

Regards,

Leif

On Fri, Jul 29, 2016 at 05:12:49PM +0000, Tim Lewis wrote:
It appears that this file is not actually used. It is only
referenced in the [Rule.Common.UEFI_DRIVER.NATIVE_BINARY] rule in
PlatformPkg.fdf. A little further research shows that an alternate
method was used for the actual GOP binary (see below). A grep of the
entire tree shows that no one uses this rule NATIVE_BINARY. So it
looks like it can just be cut out.

BTW, the downside of the method used for the binary version of the
GOP driver, is that those drivers cannot use PCDs, since the PCD
database is created based on references in the .inf. GOP works
because it is pure UEFI and (therefore) doesn't use PCDs.

Tim

FILE DRIVER = FF0C8745-3270-4439-B74F-3E45F8C77064 {
SECTION DXE_DEPEX_EXP = {gPlatformGOPPolicyGuid}
SECTION PE32 =
Vlv2MiscBinariesPkg/GOP/7.2.1011/RELEASE_VS2008x86/$(DXE_ARCHITECTURE)/IntelGopDriver.e
fi
SECTION UI = "IntelGopDriver"
}

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Leif
Lindholm
Sent: Friday, July 29, 2016 9:45 AM
To: Laszlo Ersek <lersek@redhat.com>
Cc: michael.d.kinney@intel.com; Jordan Justen <jordan.l.justen@intel.com>;
edk2-
devel@ml01.01.org; Andrew Fish <afish@apple.com>
Subject: Re: [edk2] [PATCH] add top-level .gitattributes file, dealing with
.depex

On Thu, Jul 07, 2016 at 05:03:13PM +0200, Laszlo Ersek wrote:
On 07/07/16 16:24, Leif Lindholm wrote:
Git tends to see .depex files as text, causing hideous patches being
generated (and breaking PatchCheck.py).

Add a .gitattributes file instructing git to treat them as binary.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
---
.gitattributes | 1 +
1 file changed, 1 insertion(+)
create mode 100644 .gitattributes

diff --git a/.gitattributes b/.gitattributes new file mode 100644
index 0000000..2d8a45b
--- /dev/null
+++ b/.gitattributes
@@ -0,0 +1 @@
+*.depex binary
What generates .depex files? I've never seen any.

Also, unless you add .depex files with "git add" to the set of tracked
files, no patches / diffs should cover them. What am I missing? :)

... Hm, after

$ find . -iname "*.depex"

I see .depex files in Build/ (which should be ignored altogether), and

./Vlv2TbltDevicePkg/IntelGopDepex/IntelGopDriver.depex

Why does that file exist in the tree? Let me see... git log says nothing
relevant
(the file dates back to commit 3cbfba02fef9, "Upload BSD-licensed
Vlv2TbltDevicePkg and
Vlv2DeviceRefCodePkg to").

Grepping the tree for the filename itself leads to:

Vlv2TbltDevicePkg/PlatformPkg.fdf: DXE_DEPEX DXE_DEPEX Optional
$(WORKSPACE)/$(PLATFORM_PACKAGE)/IntelGopDepex/IntelGopDriver.depex
Vlv2TbltDevicePkg/PlatformPkgGcc.fdf: DXE_DEPEX DXE_DEPEX Optional
$(WORKSPACE)/$(PLATFORM_PACKAGE)/IntelGopDepex/IntelGopDriver.depex

Do these rules exist to override the DEPEX sections of binary-only modules?
If
so: that's horrible.

Anyway, given that edk2 contains at least one .depex file, and your patch
is
correct according to <https://git-scm.com/book/en/v2/Customizing-Git-Git-
Attributes>:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Thanks!

I had hoped for comments from someone else on cc, since we don't have any
Maintainers.txt entry for the top level directory :)

But if I don't hear anything before Monday, I'll push it then.

Regards,

Leif

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Tim Lewis
 

Jordan --

As a company that delivers a lot of mixed binary/source builds, we see .depex as actually important for ease of maintenance. The .fdf syntax can work, as you mention, but it is actually requires an extra step for those of us maintaining binary modules. Why? Because .depex is derived from the .inf of the module *and* the .infs of all library instances which the module is linked against. While this can be tracked down using a build report, it is problematic and likely to introduce hard to track bugs. Since .depex is a normal product of the source build process, it is convenient.

As for the open-source, I would only note that it is used only in the exact same cases where the module itself is delivered as a binary. In fact, it could be checked in to the tree as a complete FFS file (no .efi at all).

Thanks,

Tim

-----Original Message-----
From: Jordan Justen [mailto:jordan.l.justen@intel.com]
Sent: Monday, August 01, 2016 12:03 AM
To: Kinney, Michael D <michael.d.kinney@intel.com>; Leif Lindholm <leif.lindholm@linaro.org>; Tim Lewis <tim.lewis@insyde.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>; edk2-devel@lists.01.org; Andrew Fish <afish@apple.com>
Subject: RE: [edk2] [PATCH] add top-level .gitattributes file, dealing with .depex

On 2016-07-31 16:52:23, Kinney, Michael D wrote:
Jordan,

UEFI Drivers distributed as binaries do not need depex sections.

PI modules distributed as binaries do require a .depex binary.
They may require a depex, but, as mentioned below, they can also add it directly in the .fdf. As it stands, apparently we have 1 .depex file in the tree, and it is unused.

Aside from this, under what conditions would we take such binaries into the EDK II tree? Today we have the ShellPkg and FatPkg binaries in the EDK II tree, but we recently discussed removing even those.

For an open source project, I think it is best to not have pre-built binaries, unless there is some very compelling reason. Previously there was some license funniness on FatPkg, but now that is gone. If it took an hour to build FatPkg, then that might also be something to discuss. :)

I don't think adding the .gitattributes is really a problem, aside from the fact that it implies that we might actually have a reason to add a .depex file to the source tree.

-Jordan

So I would recommend .depex binary files be treated the same as binary
.efi files by GIT. So it does sound like we need some minor updates
to GIT attributes.

If we have an example of a binary module that is providing more binary
leaf sections than are actually required and/or used, then yes, the
binary module should be cleaned up to remove the unused content.

Thanks,

Mike

-----Original Message-----
From: Justen, Jordan L
Sent: Sunday, July 31, 2016 3:58 PM
To: Leif Lindholm <leif.lindholm@linaro.org>; Tim Lewis
<tim.lewis@insyde.com>
Cc: Laszlo Ersek <lersek@redhat.com>; Kinney, Michael D
<michael.d.kinney@intel.com>; edk2-devel@ml01.01.org
<edk2-devel@lists.01.org>; Andrew Fish <afish@apple.com>
Subject: Re: [edk2] [PATCH] add top-level .gitattributes file,
dealing with .depex

On 2016-07-30 11:33:43, Leif Lindholm wrote:
Hi Tim,

Thanks for the warning, and investigation.

Does this mean that you think we should ban the inclusion of
.depex files in EDK2, including future platform trees?
I don't know about banning it, but at least we could wait for
someone to make a reasonable argument why they are needed.

Even for binary only modules, it looks like the fdf method outlined
below is preferable to a pre-built .depex.

If (at a future point) the reason for using a .depex is to support a
binary only module in a supposedly open platform under EDK II, then
I guess we can decide if that is a good idea at that point.

Should we delete this one unused .depex from the tree?

-Jordan

(If not, this patch is
still needed for git to work predictably with these files.)

Regards,

Leif

On Fri, Jul 29, 2016 at 05:12:49PM +0000, Tim Lewis wrote:
It appears that this file is not actually used. It is only
referenced in the [Rule.Common.UEFI_DRIVER.NATIVE_BINARY] rule
in PlatformPkg.fdf. A little further research shows that an
alternate method was used for the actual GOP binary (see below).
A grep of the entire tree shows that no one uses this rule
NATIVE_BINARY. So it looks like it can just be cut out.

BTW, the downside of the method used for the binary version of
the GOP driver, is that those drivers cannot use PCDs, since the
PCD database is created based on references in the .inf. GOP
works because it is pure UEFI and (therefore) doesn't use PCDs.

Tim

FILE DRIVER = FF0C8745-3270-4439-B74F-3E45F8C77064 {
SECTION DXE_DEPEX_EXP = {gPlatformGOPPolicyGuid}
SECTION PE32 =
Vlv2MiscBinariesPkg/GOP/7.2.1011/RELEASE_VS2008x86/$(DXE_ARCHITECTUR
E)/IntelGopDriver.e
fi
SECTION UI = "IntelGopDriver"
}

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On
Behalf Of Leif
Lindholm
Sent: Friday, July 29, 2016 9:45 AM
To: Laszlo Ersek <lersek@redhat.com>
Cc: michael.d.kinney@intel.com; Jordan Justen
<jordan.l.justen@intel.com>; edk2-
devel@ml01.01.org; Andrew Fish <afish@apple.com>
Subject: Re: [edk2] [PATCH] add top-level .gitattributes file,
dealing with .depex

On Thu, Jul 07, 2016 at 05:03:13PM +0200, Laszlo Ersek wrote:
On 07/07/16 16:24, Leif Lindholm wrote:
Git tends to see .depex files as text, causing hideous
patches being generated (and breaking PatchCheck.py).

Add a .gitattributes file instructing git to treat them as binary.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
---
.gitattributes | 1 +
1 file changed, 1 insertion(+) create mode 100644
.gitattributes

diff --git a/.gitattributes b/.gitattributes new file mode
100644 index 0000000..2d8a45b
--- /dev/null
+++ b/.gitattributes
@@ -0,0 +1 @@
+*.depex binary
What generates .depex files? I've never seen any.

Also, unless you add .depex files with "git add" to the set of
tracked files, no patches / diffs should cover them. What am I
missing? :)

... Hm, after

$ find . -iname "*.depex"

I see .depex files in Build/ (which should be ignored
altogether), and

./Vlv2TbltDevicePkg/IntelGopDepex/IntelGopDriver.depex

Why does that file exist in the tree? Let me see... git log
says nothing relevant
(the file dates back to commit 3cbfba02fef9, "Upload BSD-licensed
Vlv2TbltDevicePkg and Vlv2DeviceRefCodePkg to").

Grepping the tree for the filename itself leads to:

Vlv2TbltDevicePkg/PlatformPkg.fdf: DXE_DEPEX DXE_DEPEX Optional
$(WORKSPACE)/$(PLATFORM_PACKAGE)/IntelGopDepex/IntelGopDriver.depex
Vlv2TbltDevicePkg/PlatformPkgGcc.fdf: DXE_DEPEX DXE_DEPEX Optional
$(WORKSPACE)/$(PLATFORM_PACKAGE)/IntelGopDepex/IntelGopDriver.depex

Do these rules exist to override the DEPEX sections of
binary-only modules? If
so: that's horrible.

Anyway, given that edk2 contains at least one .depex file, and
your patch is
correct according to <https://git-scm.com/book/en/v2/Customizing-Git-Git-Attributes>:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Thanks!

I had hoped for comments from someone else on cc, since we don't
have any
Maintainers.txt entry for the top level directory :)

But if I don't hear anything before Monday, I'll push it then.

Regards,

Leif

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Liming Gao
 

Tim:
FDF syntax doesn't support .ffs.

Thanks
Liming

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
Tim Lewis
Sent: Tuesday, August 02, 2016 1:03 AM
To: Justen, Jordan L <jordan.l.justen@intel.com>; Kinney, Michael D
<michael.d.kinney@intel.com>; Leif Lindholm <leif.lindholm@linaro.org>;
Kinney, Michael D <michael.d.kinney@intel.com>
Cc: edk2-devel@lists.01.org; Laszlo Ersek <lersek@redhat.com>; Andrew Fish
<afish@apple.com>
Subject: Re: [edk2] [PATCH] add top-level .gitattributes file, dealing
with .depex

Jordan --

As a company that delivers a lot of mixed binary/source builds, we
see .depex as actually important for ease of maintenance. The .fdf syntax
can work, as you mention, but it is actually requires an extra step for those of
us maintaining binary modules. Why? Because .depex is derived from the .inf
of the module *and* the .infs of all library instances which the module is
linked against. While this can be tracked down using a build report, it is
problematic and likely to introduce hard to track bugs. Since .depex is a
normal product of the source build process, it is convenient.

As for the open-source, I would only note that it is used only in the exact
same cases where the module itself is delivered as a binary. In fact, it could
be checked in to the tree as a complete FFS file (no .efi at all).

Thanks,

Tim




-----Original Message-----
From: Jordan Justen [mailto:jordan.l.justen@intel.com]
Sent: Monday, August 01, 2016 12:03 AM
To: Kinney, Michael D <michael.d.kinney@intel.com>; Leif Lindholm
<leif.lindholm@linaro.org>; Tim Lewis <tim.lewis@insyde.com>; Kinney,
Michael D <michael.d.kinney@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>; edk2-devel@lists.01.org; Andrew Fish
<afish@apple.com>
Subject: RE: [edk2] [PATCH] add top-level .gitattributes file, dealing
with .depex

On 2016-07-31 16:52:23, Kinney, Michael D wrote:
Jordan,

UEFI Drivers distributed as binaries do not need depex sections.

PI modules distributed as binaries do require a .depex binary.
They may require a depex, but, as mentioned below, they can also add it
directly in the .fdf. As it stands, apparently we have 1 .depex file in the tree,
and it is unused.

Aside from this, under what conditions would we take such binaries into the
EDK II tree? Today we have the ShellPkg and FatPkg binaries in the EDK II tree,
but we recently discussed removing even those.

For an open source project, I think it is best to not have pre-built binaries,
unless there is some very compelling reason. Previously there was some
license funniness on FatPkg, but now that is gone. If it took an hour to build
FatPkg, then that might also be something to discuss. :)

I don't think adding the .gitattributes is really a problem, aside from the fact
that it implies that we might actually have a reason to add a .depex file to the
source tree.

-Jordan

So I would recommend .depex binary files be treated the same as binary
.efi files by GIT. So it does sound like we need some minor updates
to GIT attributes.

If we have an example of a binary module that is providing more binary
leaf sections than are actually required and/or used, then yes, the
binary module should be cleaned up to remove the unused content.

Thanks,

Mike

-----Original Message-----
From: Justen, Jordan L
Sent: Sunday, July 31, 2016 3:58 PM
To: Leif Lindholm <leif.lindholm@linaro.org>; Tim Lewis
<tim.lewis@insyde.com>
Cc: Laszlo Ersek <lersek@redhat.com>; Kinney, Michael D
<michael.d.kinney@intel.com>; edk2-devel@ml01.01.org
<edk2-devel@lists.01.org>; Andrew Fish <afish@apple.com>
Subject: Re: [edk2] [PATCH] add top-level .gitattributes file,
dealing with .depex

On 2016-07-30 11:33:43, Leif Lindholm wrote:
Hi Tim,

Thanks for the warning, and investigation.

Does this mean that you think we should ban the inclusion of
.depex files in EDK2, including future platform trees?
I don't know about banning it, but at least we could wait for
someone to make a reasonable argument why they are needed.

Even for binary only modules, it looks like the fdf method outlined
below is preferable to a pre-built .depex.

If (at a future point) the reason for using a .depex is to support a
binary only module in a supposedly open platform under EDK II, then
I guess we can decide if that is a good idea at that point.

Should we delete this one unused .depex from the tree?

-Jordan

(If not, this patch is
still needed for git to work predictably with these files.)

Regards,

Leif

On Fri, Jul 29, 2016 at 05:12:49PM +0000, Tim Lewis wrote:
It appears that this file is not actually used. It is only
referenced in the [Rule.Common.UEFI_DRIVER.NATIVE_BINARY] rule
in PlatformPkg.fdf. A little further research shows that an
alternate method was used for the actual GOP binary (see below).
A grep of the entire tree shows that no one uses this rule
NATIVE_BINARY. So it looks like it can just be cut out.

BTW, the downside of the method used for the binary version of
the GOP driver, is that those drivers cannot use PCDs, since the
PCD database is created based on references in the .inf. GOP
works because it is pure UEFI and (therefore) doesn't use PCDs.

Tim

FILE DRIVER = FF0C8745-3270-4439-B74F-3E45F8C77064 {
SECTION DXE_DEPEX_EXP = {gPlatformGOPPolicyGuid}
SECTION PE32 =
Vlv2MiscBinariesPkg/GOP/7.2.1011/RELEASE_VS2008x86/$(DXE_ARCHITECT
UR
E)/IntelGopDriver.e
fi
SECTION UI = "IntelGopDriver"
}

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On
Behalf Of Leif
Lindholm
Sent: Friday, July 29, 2016 9:45 AM
To: Laszlo Ersek <lersek@redhat.com>
Cc: michael.d.kinney@intel.com; Jordan Justen
<jordan.l.justen@intel.com>; edk2-
devel@ml01.01.org; Andrew Fish <afish@apple.com>
Subject: Re: [edk2] [PATCH] add top-level .gitattributes file,
dealing with .depex

On Thu, Jul 07, 2016 at 05:03:13PM +0200, Laszlo Ersek wrote:
On 07/07/16 16:24, Leif Lindholm wrote:
Git tends to see .depex files as text, causing hideous
patches being generated (and breaking PatchCheck.py).

Add a .gitattributes file instructing git to treat them as binary.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
---
.gitattributes | 1 +
1 file changed, 1 insertion(+) create mode 100644
.gitattributes

diff --git a/.gitattributes b/.gitattributes new file mode
100644 index 0000000..2d8a45b
--- /dev/null
+++ b/.gitattributes
@@ -0,0 +1 @@
+*.depex binary
What generates .depex files? I've never seen any.

Also, unless you add .depex files with "git add" to the set of
tracked files, no patches / diffs should cover them. What am I
missing? :)

... Hm, after

$ find . -iname "*.depex"

I see .depex files in Build/ (which should be ignored
altogether), and

./Vlv2TbltDevicePkg/IntelGopDepex/IntelGopDriver.depex

Why does that file exist in the tree? Let me see... git log
says nothing relevant
(the file dates back to commit 3cbfba02fef9, "Upload BSD-licensed
Vlv2TbltDevicePkg and Vlv2DeviceRefCodePkg to").

Grepping the tree for the filename itself leads to:

Vlv2TbltDevicePkg/PlatformPkg.fdf: DXE_DEPEX DXE_DEPEX
Optional
$(WORKSPACE)/$(PLATFORM_PACKAGE)/IntelGopDepex/IntelGopDriver.de
pex
Vlv2TbltDevicePkg/PlatformPkgGcc.fdf: DXE_DEPEX DXE_DEPEX
Optional
$(WORKSPACE)/$(PLATFORM_PACKAGE)/IntelGopDepex/IntelGopDriver.de
pex

Do these rules exist to override the DEPEX sections of
binary-only modules? If
so: that's horrible.

Anyway, given that edk2 contains at least one .depex file, and
your patch is
correct according to <https://git-scm.com/book/en/v2/Customizing-Git-
Git-Attributes>:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Thanks!

I had hoped for comments from someone else on cc, since we don't
have any
Maintainers.txt entry for the top level directory :)

But if I don't hear anything before Monday, I'll push it then.

Regards,

Leif

_______________________________________________
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