public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* Re: [Bug 164] Add the build option "/D DISABLE_NEW_DEPRECATED_INTERFACES" in package DSC files
       [not found] ` <bug-164-63-L8k0GFC2io@https.bugzilla.tianocore.org/>
@ 2016-10-21 19:37   ` Ard Biesheuvel
  2016-10-21 19:41     ` Michael Zimmermann
  2016-10-21 19:58     ` Jordan Justen
  0 siblings, 2 replies; 16+ messages in thread
From: Ard Biesheuvel @ 2016-10-21 19:37 UTC (permalink / raw)
  To: edk2-devel-01, Kinney, Michael D, Leif Lindholm, afish@apple.com
  Cc: Laszlo Ersek, Gao, Liming, Zhu, Yonghong

I don't remember seeing any discussion regarding
DISABLE_NEW_DEPRECATED_INTERFACES on the list, so I am a bit surprised
seeing these bugs being filed and assigned.

Before making any such changes, I would like a strong commitment from
other package owners that deprecating an interface brings along with
it the responsibility to update all existing callers, otherwise
setting this define will only result in more breakage, and ARM has
seen its share of inadvertent breakage in the past when changes to
core code were made without taking other architectures into account.



On 21 October 2016 at 02:21,  <bugzilla-daemon@bugzilla.tianocore.org> wrote:
> https://bugzilla.tianocore.org/show_bug.cgi?id=164
>
> yonghong.zhu@intel.com changed:
>
>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>            Priority|Lowest                      |Normal
>              Status|UNCONFIRMED                 |CONFIRMED
>            Assignee|michael.d.kinney@intel.com  |ard.biesheuvel@linaro.org
>      Ever confirmed|0                           |1
>      Release(s) the|                            |EDK II Trunk
>      issues must be|                            |
>               fixed|                            |
>
> --- Comment #1 from yonghong.zhu@intel.com ---
> Assign to Package owner.
>
> --
> You are receiving this mail because:
> You are the assignee for the bug.


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

* Re: [Bug 164] Add the build option "/D DISABLE_NEW_DEPRECATED_INTERFACES" in package DSC files
  2016-10-21 19:37   ` [Bug 164] Add the build option "/D DISABLE_NEW_DEPRECATED_INTERFACES" in package DSC files Ard Biesheuvel
@ 2016-10-21 19:41     ` Michael Zimmermann
  2016-10-21 19:58     ` Jordan Justen
  1 sibling, 0 replies; 16+ messages in thread
From: Michael Zimmermann @ 2016-10-21 19:41 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: edk2-devel-01, Kinney, Michael D, Leif Lindholm, afish@apple.com,
	Laszlo Ersek, Gao, Liming

I actually sent a patch for Arm some time ago:
https://lists.01.org/pipermail/edk2-devel/2016-August/000489.html

Thanks
Michael

On Fri, Oct 21, 2016 at 9:37 PM, Ard Biesheuvel <ard.biesheuvel@linaro.org>
wrote:

> I don't remember seeing any discussion regarding
> DISABLE_NEW_DEPRECATED_INTERFACES on the list, so I am a bit surprised
> seeing these bugs being filed and assigned.
>
> Before making any such changes, I would like a strong commitment from
> other package owners that deprecating an interface brings along with
> it the responsibility to update all existing callers, otherwise
> setting this define will only result in more breakage, and ARM has
> seen its share of inadvertent breakage in the past when changes to
> core code were made without taking other architectures into account.
>
>
>
> On 21 October 2016 at 02:21,  <bugzilla-daemon@bugzilla.tianocore.org>
> wrote:
> > https://bugzilla.tianocore.org/show_bug.cgi?id=164
> >
> > yonghong.zhu@intel.com changed:
> >
> >            What    |Removed                     |Added
> > ------------------------------------------------------------
> ----------------
> >            Priority|Lowest                      |Normal
> >              Status|UNCONFIRMED                 |CONFIRMED
> >            Assignee|michael.d.kinney@intel.com  |
> ard.biesheuvel@linaro.org
> >      Ever confirmed|0                           |1
> >      Release(s) the|                            |EDK II Trunk
> >      issues must be|                            |
> >               fixed|                            |
> >
> > --- Comment #1 from yonghong.zhu@intel.com ---
> > Assign to Package owner.
> >
> > --
> > You are receiving this mail because:
> > You are the assignee for the bug.
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
>


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

* Re: [Bug 164] Add the build option "/D DISABLE_NEW_DEPRECATED_INTERFACES" in package DSC files
  2016-10-21 19:37   ` [Bug 164] Add the build option "/D DISABLE_NEW_DEPRECATED_INTERFACES" in package DSC files Ard Biesheuvel
  2016-10-21 19:41     ` Michael Zimmermann
@ 2016-10-21 19:58     ` Jordan Justen
  2016-10-21 20:14       ` Laszlo Ersek
  2016-10-21 20:20       ` Andrew Fish
  1 sibling, 2 replies; 16+ messages in thread
From: Jordan Justen @ 2016-10-21 19:58 UTC (permalink / raw)
  To: Ard Biesheuvel, edk2-devel-01, Kinney, Michael D, Leif Lindholm,
	afish@apple.com
  Cc: Laszlo Ersek, Gao, Liming

On 2016-10-21 12:37:21, Ard Biesheuvel wrote:
> I don't remember seeing any discussion regarding
> DISABLE_NEW_DEPRECATED_INTERFACES on the list, so I am a bit surprised
> seeing these bugs being filed and assigned.
>

I agree.

Also, the terminology seems confusing. 'new deprecated' seems like a
contradiction. I guess it means 'newly deprecated', but that seems
like a term that is quickly going to become obsolete. Soon there will
be old deprecated items that are disabled with this switch.
DISABLE_DEPRECATED_INTERFACES sounds better.

But, shouldn't we have platforms opt-in to using the deprecated
interfaces rather than adding DISABLE_NEW_DEPRECATED_INTERFACES to the
build command line for every EDK II platform?

Not using deprecated items should be the default for EDK II platforms.
If a platform has to opt-in to the deprecated content in their .dsc,
then it is obvious that they are relying on deprecated functionality.

So, I guess I'd propose adding ENABLE_DEPRECATED_INTERFACES instead.

-Jordan

> Before making any such changes, I would like a strong commitment from
> other package owners that deprecating an interface brings along with
> it the responsibility to update all existing callers, otherwise
> setting this define will only result in more breakage, and ARM has
> seen its share of inadvertent breakage in the past when changes to
> core code were made without taking other architectures into account.
>
> On 21 October 2016 at 02:21,  <bugzilla-daemon@bugzilla.tianocore.org> wrote:
> > https://bugzilla.tianocore.org/show_bug.cgi?id=164
> >
> > yonghong.zhu@intel.com changed:
> >
> >            What    |Removed                     |Added
> > ----------------------------------------------------------------------------
> >            Priority|Lowest                      |Normal
> >              Status|UNCONFIRMED                 |CONFIRMED
> >            Assignee|michael.d.kinney@intel.com  |ard.biesheuvel@linaro.org
> >      Ever confirmed|0                           |1
> >      Release(s) the|                            |EDK II Trunk
> >      issues must be|                            |
> >               fixed|                            |
> >
> > --- Comment #1 from yonghong.zhu@intel.com ---
> > Assign to Package owner.
> >
> > --
> > You are receiving this mail because:
> > You are the assignee for the bug.
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


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

* Re: [Bug 164] Add the build option "/D DISABLE_NEW_DEPRECATED_INTERFACES" in package DSC files
  2016-10-21 19:58     ` Jordan Justen
@ 2016-10-21 20:14       ` Laszlo Ersek
  2016-10-21 20:19         ` Ard Biesheuvel
  2016-10-21 20:20       ` Andrew Fish
  1 sibling, 1 reply; 16+ messages in thread
From: Laszlo Ersek @ 2016-10-21 20:14 UTC (permalink / raw)
  To: Jordan Justen, Ard Biesheuvel, edk2-devel-01, Kinney, Michael D,
	Leif Lindholm, afish@apple.com
  Cc: Gao, Liming

On 10/21/16 21:58, Jordan Justen wrote:
> On 2016-10-21 12:37:21, Ard Biesheuvel wrote:
>> I don't remember seeing any discussion regarding
>> DISABLE_NEW_DEPRECATED_INTERFACES on the list, so I am a bit surprised
>> seeing these bugs being filed and assigned.
>>
> 
> I agree.
> 
> Also, the terminology seems confusing. 'new deprecated' seems like a
> contradiction. I guess it means 'newly deprecated', but that seems
> like a term that is quickly going to become obsolete. Soon there will
> be old deprecated items that are disabled with this switch.
> DISABLE_DEPRECATED_INTERFACES sounds better.
> 
> But, shouldn't we have platforms opt-in to using the deprecated
> interfaces rather than adding DISABLE_NEW_DEPRECATED_INTERFACES to the
> build command line for every EDK II platform?
> 
> Not using deprecated items should be the default for EDK II platforms.
> If a platform has to opt-in to the deprecated content in their .dsc,
> then it is obvious that they are relying on deprecated functionality.
> 
> So, I guess I'd propose adding ENABLE_DEPRECATED_INTERFACES instead.

I'm about to post a ~20-part series for OvmfPkg and ArmVirtPkg together
that solves these BZs. :/ The DISABLE_NEW_DEPRECATED_INTERFACES feature
test macro is already being used in three MdePkg library class header
files (and a number of library instances), so I thought it was a done deal.

I don't want to stifle the discussion of course, but at this point I
think I will post the patches for review.

Thanks
Laszlo

> -Jordan
> 
>> Before making any such changes, I would like a strong commitment from
>> other package owners that deprecating an interface brings along with
>> it the responsibility to update all existing callers, otherwise
>> setting this define will only result in more breakage, and ARM has
>> seen its share of inadvertent breakage in the past when changes to
>> core code were made without taking other architectures into account.
>>
>> On 21 October 2016 at 02:21,  <bugzilla-daemon@bugzilla.tianocore.org> wrote:
>>> https://bugzilla.tianocore.org/show_bug.cgi?id=164
>>>
>>> yonghong.zhu@intel.com changed:
>>>
>>>            What    |Removed                     |Added
>>> ----------------------------------------------------------------------------
>>>            Priority|Lowest                      |Normal
>>>              Status|UNCONFIRMED                 |CONFIRMED
>>>            Assignee|michael.d.kinney@intel.com  |ard.biesheuvel@linaro.org
>>>      Ever confirmed|0                           |1
>>>      Release(s) the|                            |EDK II Trunk
>>>      issues must be|                            |
>>>               fixed|                            |
>>>
>>> --- Comment #1 from yonghong.zhu@intel.com ---
>>> Assign to Package owner.
>>>
>>> --
>>> You are receiving this mail because:
>>> You are the assignee for the bug.
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel



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

* Re: [Bug 164] Add the build option "/D DISABLE_NEW_DEPRECATED_INTERFACES" in package DSC files
  2016-10-21 20:14       ` Laszlo Ersek
@ 2016-10-21 20:19         ` Ard Biesheuvel
  2016-10-21 20:40           ` Laszlo Ersek
  0 siblings, 1 reply; 16+ messages in thread
From: Ard Biesheuvel @ 2016-10-21 20:19 UTC (permalink / raw)
  To: Laszlo Ersek
  Cc: Jordan Justen, edk2-devel-01, Kinney, Michael D, Leif Lindholm,
	afish@apple.com, Gao, Liming

On 21 October 2016 at 21:14, Laszlo Ersek <lersek@redhat.com> wrote:
> On 10/21/16 21:58, Jordan Justen wrote:
>> On 2016-10-21 12:37:21, Ard Biesheuvel wrote:
>>> I don't remember seeing any discussion regarding
>>> DISABLE_NEW_DEPRECATED_INTERFACES on the list, so I am a bit surprised
>>> seeing these bugs being filed and assigned.
>>>
>>
>> I agree.
>>
>> Also, the terminology seems confusing. 'new deprecated' seems like a
>> contradiction. I guess it means 'newly deprecated', but that seems
>> like a term that is quickly going to become obsolete. Soon there will
>> be old deprecated items that are disabled with this switch.
>> DISABLE_DEPRECATED_INTERFACES sounds better.
>>
>> But, shouldn't we have platforms opt-in to using the deprecated
>> interfaces rather than adding DISABLE_NEW_DEPRECATED_INTERFACES to the
>> build command line for every EDK II platform?
>>
>> Not using deprecated items should be the default for EDK II platforms.
>> If a platform has to opt-in to the deprecated content in their .dsc,
>> then it is obvious that they are relying on deprecated functionality.
>>
>> So, I guess I'd propose adding ENABLE_DEPRECATED_INTERFACES instead.
>
> I'm about to post a ~20-part series for OvmfPkg and ArmVirtPkg together
> that solves these BZs. :/ The DISABLE_NEW_DEPRECATED_INTERFACES feature
> test macro is already being used in three MdePkg library class header
> files (and a number of library instances), so I thought it was a done deal.
>
> I don't want to stifle the discussion of course, but at this point I
> think I will post the patches for review.
>

Yes, please. Removing uses of deprecated interfaces is something we
should do anyway. What we shouldn't do is configure our platforms in
such a way that additional future deprecation automatically break the
build, unless we have a strong commitment from the other package
owners that they will ensure that this does not happen.


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

* Re: [Bug 164] Add the build option "/D DISABLE_NEW_DEPRECATED_INTERFACES" in package DSC files
  2016-10-21 19:58     ` Jordan Justen
  2016-10-21 20:14       ` Laszlo Ersek
@ 2016-10-21 20:20       ` Andrew Fish
  2016-10-21 20:39         ` Jordan Justen
  1 sibling, 1 reply; 16+ messages in thread
From: Andrew Fish @ 2016-10-21 20:20 UTC (permalink / raw)
  To: Jordan Justen
  Cc: Ard Biesheuvel, edk2-devel-01, Mike Kinney, Leif Lindholm,
	Laszlo Ersek, Gao, Liming


> On Oct 21, 2016, at 12:58 PM, Jordan Justen <jordan.l.justen@intel.com> wrote:
> 
> On 2016-10-21 12:37:21, Ard Biesheuvel wrote:
>> I don't remember seeing any discussion regarding
>> DISABLE_NEW_DEPRECATED_INTERFACES on the list, so I am a bit surprised
>> seeing these bugs being filed and assigned.
>> 
> 
> I agree.
> 
> Also, the terminology seems confusing. 'new deprecated' seems like a
> contradiction. I guess it means 'newly deprecated', but that seems
> like a term that is quickly going to become obsolete. Soon there will
> be old deprecated items that are disabled with this switch.
> DISABLE_DEPRECATED_INTERFACES sounds better.
> 
> But, shouldn't we have platforms opt-in to using the deprecated
> interfaces rather than adding DISABLE_NEW_DEPRECATED_INTERFACES to the
> build command line for every EDK II platform?
> 
> Not using deprecated items should be the default for EDK II platforms.
> If a platform has to opt-in to the deprecated content in their .dsc,
> then it is obvious that they are relying on deprecated functionality.
> 
> So, I guess I'd propose adding ENABLE_DEPRECATED_INTERFACES instead.
> 

Jordan,

I think it depends on your point of view. If you have a platform that works and you update the edk2 revision you would expect it to still work. Thus the option is to DISABLE_DEPRECATED_INTERFACES as that maintains backward compatibility. 

I think it makes total sense to turn on DISABLE_DEPRECATED_INTERFACES on all the open source edk2 platform as soon as possible so all the open source code is following current best practices.

Not to mention it would probably be a really good idea to give all the downstream folks a long lead time about the plan of making a non backward compatible change. 

Thanks,

Andrew Fish


> -Jordan
> 
>> Before making any such changes, I would like a strong commitment from
>> other package owners that deprecating an interface brings along with
>> it the responsibility to update all existing callers, otherwise
>> setting this define will only result in more breakage, and ARM has
>> seen its share of inadvertent breakage in the past when changes to
>> core code were made without taking other architectures into account.
>> 
>> On 21 October 2016 at 02:21,  <bugzilla-daemon@bugzilla.tianocore.org> wrote:
>>> https://bugzilla.tianocore.org/show_bug.cgi?id=164
>>> 
>>> yonghong.zhu@intel.com changed:
>>> 
>>>           What    |Removed                     |Added
>>> ----------------------------------------------------------------------------
>>>           Priority|Lowest                      |Normal
>>>             Status|UNCONFIRMED                 |CONFIRMED
>>>           Assignee|michael.d.kinney@intel.com  |ard.biesheuvel@linaro.org
>>>     Ever confirmed|0                           |1
>>>     Release(s) the|                            |EDK II Trunk
>>>     issues must be|                            |
>>>              fixed|                            |
>>> 
>>> --- Comment #1 from yonghong.zhu@intel.com ---
>>> Assign to Package owner.
>>> 
>>> --
>>> You are receiving this mail because:
>>> You are the assignee for the bug.
>> _______________________________________________
>> 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>


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

* Re: [Bug 164] Add the build option "/D DISABLE_NEW_DEPRECATED_INTERFACES" in package DSC files
  2016-10-21 20:20       ` Andrew Fish
@ 2016-10-21 20:39         ` Jordan Justen
  2016-10-21 20:54           ` Andrew Fish
  2016-10-21 21:02           ` Laszlo Ersek
  0 siblings, 2 replies; 16+ messages in thread
From: Jordan Justen @ 2016-10-21 20:39 UTC (permalink / raw)
  To: Andrew Fish
  Cc: Ard Biesheuvel, edk2-devel-01, Mike Kinney, Leif Lindholm,
	Laszlo Ersek, Gao, Liming

On 2016-10-21 13:20:49, Andrew Fish wrote:
>      On Oct 21, 2016, at 12:58 PM, Jordan Justen <jordan.l.justen@intel.com>
>      wrote:
>      On 2016-10-21 12:37:21, Ard Biesheuvel wrote:
> 
>        I don't remember seeing any discussion regarding
>        DISABLE_NEW_DEPRECATED_INTERFACES on the list, so I am a bit surprised
>        seeing these bugs being filed and assigned.
> 
>      I agree.
> 
>      Also, the terminology seems confusing. 'new deprecated' seems like a
>      contradiction. I guess it means 'newly deprecated', but that seems
>      like a term that is quickly going to become obsolete. Soon there will
>      be old deprecated items that are disabled with this switch.
>      DISABLE_DEPRECATED_INTERFACES sounds better.
> 
>      But, shouldn't we have platforms opt-in to using the deprecated
>      interfaces rather than adding DISABLE_NEW_DEPRECATED_INTERFACES to the
>      build command line for every EDK II platform?
> 
>      Not using deprecated items should be the default for EDK II platforms.
>      If a platform has to opt-in to the deprecated content in their .dsc,
>      then it is obvious that they are relying on deprecated functionality.
> 
>      So, I guess I'd propose adding ENABLE_DEPRECATED_INTERFACES instead.
> 
>    Jordan,
>    I think it depends on your point of view. If you have a platform that
>    works and you update the edk2 revision you would expect it to still work.

I think this is what UDK is for. If you want to depend directly on EDK
II, then you'll see less stability.

>    Thus the option is to DISABLE_DEPRECATED_INTERFACES as that maintains
>    backward compatibility.

In order to support UDK releases, maybe ENABLE_UDK2014_INTERFACES would be
something to consider. Or ENABLE_UDK_INTERFACE=2014 so we can use <=.

But, I still think that EDK II platforms (as a goal) should represent
the best, cleanest examples of using EDK II. And, I think having every
platform accumulate cruft like CFLAGS to disable deprecated interfaces
works against that goal.

Another point. What about when we want to deprecate more interfaces?
Oh know, we better not break platforms that only specified
DISABLE_NEW_DEPRECATED_INTERFACES! Let's add
DISABLE_NEW_DEPRECATED_INTERFACES2! :)

-Jordan

>    I think it makes total sense to turn on DISABLE_DEPRECATED_INTERFACES on
>    all the open source edk2 platform as soon as possible so all the open
>    source code is following current best practices.
>    Not to mention it would probably be a really good idea to give all the
>    downstream folks a long lead time about the plan of making a non backward
>    compatible change. 
>    Thanks,
>    Andrew Fish
> 
>      -Jordan
> 
>        Before making any such changes, I would like a strong commitment from
>        other package owners that deprecating an interface brings along with
>        it the responsibility to update all existing callers, otherwise
>        setting this define will only result in more breakage, and ARM has
>        seen its share of inadvertent breakage in the past when changes to
>        core code were made without taking other architectures into account.
> 
>        On 21 October 2016 at 02:21,  <bugzilla-daemon@bugzilla.tianocore.org>
>        wrote:
> 
>          https://bugzilla.tianocore.org/show_bug.cgi?id=164
> 
>          yonghong.zhu@intel.com changed:
> 
>                    What    |Removed                     |Added
>          ----------------------------------------------------------------------------
>                    Priority|Lowest                      |Normal
>                      Status|UNCONFIRMED                 |CONFIRMED
>                    Assignee|michael.d.kinney@intel.com
>           |ard.biesheuvel@linaro.org
>              Ever confirmed|0                           |1
>              Release(s) the|                            |EDK II Trunk
>              issues must be|                            |
>                       fixed|                            |
> 
>          --- Comment #1 from yonghong.zhu@intel.com ---
>          Assign to Package owner.
> 
>          --
>          You are receiving this mail because:
>          You are the assignee for the bug.
> 
>        _______________________________________________
>        edk2-devel mailing list
>        edk2-devel@lists.01.org
>        https://lists.01.org/mailman/listinfo/edk2-devel


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

* Re: [Bug 164] Add the build option "/D DISABLE_NEW_DEPRECATED_INTERFACES" in package DSC files
  2016-10-21 20:19         ` Ard Biesheuvel
@ 2016-10-21 20:40           ` Laszlo Ersek
  2016-10-21 20:57             ` Ard Biesheuvel
  0 siblings, 1 reply; 16+ messages in thread
From: Laszlo Ersek @ 2016-10-21 20:40 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Jordan Justen, edk2-devel-01, Kinney, Michael D, Leif Lindholm,
	afish@apple.com, Gao, Liming

On 10/21/16 22:19, Ard Biesheuvel wrote:
> On 21 October 2016 at 21:14, Laszlo Ersek <lersek@redhat.com> wrote:
>> On 10/21/16 21:58, Jordan Justen wrote:
>>> On 2016-10-21 12:37:21, Ard Biesheuvel wrote:
>>>> I don't remember seeing any discussion regarding
>>>> DISABLE_NEW_DEPRECATED_INTERFACES on the list, so I am a bit surprised
>>>> seeing these bugs being filed and assigned.
>>>>
>>>
>>> I agree.
>>>
>>> Also, the terminology seems confusing. 'new deprecated' seems like a
>>> contradiction. I guess it means 'newly deprecated', but that seems
>>> like a term that is quickly going to become obsolete. Soon there will
>>> be old deprecated items that are disabled with this switch.
>>> DISABLE_DEPRECATED_INTERFACES sounds better.
>>>
>>> But, shouldn't we have platforms opt-in to using the deprecated
>>> interfaces rather than adding DISABLE_NEW_DEPRECATED_INTERFACES to the
>>> build command line for every EDK II platform?
>>>
>>> Not using deprecated items should be the default for EDK II platforms.
>>> If a platform has to opt-in to the deprecated content in their .dsc,
>>> then it is obvious that they are relying on deprecated functionality.
>>>
>>> So, I guess I'd propose adding ENABLE_DEPRECATED_INTERFACES instead.
>>
>> I'm about to post a ~20-part series for OvmfPkg and ArmVirtPkg together
>> that solves these BZs. :/ The DISABLE_NEW_DEPRECATED_INTERFACES feature
>> test macro is already being used in three MdePkg library class header
>> files (and a number of library instances), so I thought it was a done deal.
>>
>> I don't want to stifle the discussion of course, but at this point I
>> think I will post the patches for review.
>>
> 
> Yes, please. Removing uses of deprecated interfaces is something we
> should do anyway. What we shouldn't do is configure our platforms in
> such a way that additional future deprecation automatically break the
> build, unless we have a strong commitment from the other package
> owners that they will ensure that this does not happen.
> 

Well, my expectation is that the onus of keeping the tree working is on
the patch submitter. Assume we adopt DISABLE_NEW_DEPRECATED_INTERFACES
now (weaning ourselves off the functions that are *currently* disabled
by the macro). Then whenever someone moves further functions under the
scope of the macro -- thereby possibly breaking platform code --, it's
going to be their responsibility to grep the tree for platform DSCs that
enable DISABLE_NEW_DEPRECATED_INTERFACES, and either test-build those
platforms reasonably extensively, or ask for help *in advance* (before
committing the patches).

For example, in the current work I'm about the post, I couldn't
build-test everything (no RVCT over here, for example :)) Also I don't
run Xen, so the Xen-related changes can't be functionally tested on my
side -- but, I do ask for help with testing those. Same goes for the
32-bit ARM changes (which, it turns out, Michael and myself managed to
fix separately, independently, and differently :))

It's okay to modify code one can't build or test himself/herself, but
then help should be asked for, and given too.

TL;DR: the strong commitment you speak about is the default for any open
source project, isn't it? ;)

Thanks
Laszlo


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

* Re: [Bug 164] Add the build option "/D DISABLE_NEW_DEPRECATED_INTERFACES" in package DSC files
  2016-10-21 20:39         ` Jordan Justen
@ 2016-10-21 20:54           ` Andrew Fish
  2016-10-21 20:55             ` Andrew Fish
  2016-10-21 21:02           ` Laszlo Ersek
  1 sibling, 1 reply; 16+ messages in thread
From: Andrew Fish @ 2016-10-21 20:54 UTC (permalink / raw)
  To: Jordan Justen
  Cc: Ard Biesheuvel, edk2-devel-01, Gao, Liming, Leif Lindholm,
	Mike Kinney, Laszlo Ersek


> On Oct 21, 2016, at 1:39 PM, Jordan Justen <jordan.l.justen@intel.com> wrote:
> 
> On 2016-10-21 13:20:49, Andrew Fish wrote:
>>     On Oct 21, 2016, at 12:58 PM, Jordan Justen <jordan.l.justen@intel.com>
>>     wrote:
>>     On 2016-10-21 12:37:21, Ard Biesheuvel wrote:
>> 
>>       I don't remember seeing any discussion regarding
>>       DISABLE_NEW_DEPRECATED_INTERFACES on the list, so I am a bit surprised
>>       seeing these bugs being filed and assigned.
>> 
>>     I agree.
>> 
>>     Also, the terminology seems confusing. 'new deprecated' seems like a
>>     contradiction. I guess it means 'newly deprecated', but that seems
>>     like a term that is quickly going to become obsolete. Soon there will
>>     be old deprecated items that are disabled with this switch.
>>     DISABLE_DEPRECATED_INTERFACES sounds better.
>> 
>>     But, shouldn't we have platforms opt-in to using the deprecated
>>     interfaces rather than adding DISABLE_NEW_DEPRECATED_INTERFACES to the
>>     build command line for every EDK II platform?
>> 
>>     Not using deprecated items should be the default for EDK II platforms.
>>     If a platform has to opt-in to the deprecated content in their .dsc,
>>     then it is obvious that they are relying on deprecated functionality.
>> 
>>     So, I guess I'd propose adding ENABLE_DEPRECATED_INTERFACES instead.
>> 
>>   Jordan,
>>   I think it depends on your point of view. If you have a platform that
>>   works and you update the edk2 revision you would expect it to still work.
> 
> I think this is what UDK is for. If you want to depend directly on EDK
> II, then you'll see less stability.
> 

Jordan,

Well there should be a published plan for a future UDK that this change is going to happen before we "break it" in master. Publishing the plan with the UDK does not count :). 

>>   Thus the option is to DISABLE_DEPRECATED_INTERFACES as that maintains
>>   backward compatibility.
> 
> In order to support UDK releases, maybe ENABLE_UDK2014_INTERFACES would be
> something to consider. Or ENABLE_UDK_INTERFACE=2014 so we can use <=.
> 
> But, I still think that EDK II platforms (as a goal) should represent
> the best, cleanest examples of using EDK II. And, I think having every
> platform accumulate cruft like CFLAGS to disable deprecated interfaces
> works against that goal.
> 
> Another point. What about when we want to deprecate more interfaces?
> Oh know, we better not break platforms that only specified
> DISABLE_NEW_DEPRECATED_INTERFACES! Let's add
> DISABLE_NEW_DEPRECATED_INTERFACES2! :)
> 

I think you make a very good point. How about DISABLE_2014_DEPRECATE_INTERFACES. I think that version scales, and might actually encourage cleanup as it shows when the interface first got deprecated. 

Thanks,

Andrew Fish

> -Jordan
> 
>>   I think it makes total sense to turn on DISABLE_DEPRECATED_INTERFACES on
>>   all the open source edk2 platform as soon as possible so all the open
>>   source code is following current best practices.
>>   Not to mention it would probably be a really good idea to give all the
>>   downstream folks a long lead time about the plan of making a non backward
>>   compatible change. 
>>   Thanks,
>>   Andrew Fish
>> 
>>     -Jordan
>> 
>>       Before making any such changes, I would like a strong commitment from
>>       other package owners that deprecating an interface brings along with
>>       it the responsibility to update all existing callers, otherwise
>>       setting this define will only result in more breakage, and ARM has
>>       seen its share of inadvertent breakage in the past when changes to
>>       core code were made without taking other architectures into account.
>> 
>>       On 21 October 2016 at 02:21,  <bugzilla-daemon@bugzilla.tianocore.org>
>>       wrote:
>> 
>>         https://bugzilla.tianocore.org/show_bug.cgi?id=164
>> 
>>         yonghong.zhu@intel.com changed:
>> 
>>                   What    |Removed                     |Added
>>         ----------------------------------------------------------------------------
>>                   Priority|Lowest                      |Normal
>>                     Status|UNCONFIRMED                 |CONFIRMED
>>                   Assignee|michael.d.kinney@intel.com
>>          |ard.biesheuvel@linaro.org
>>             Ever confirmed|0                           |1
>>             Release(s) the|                            |EDK II Trunk
>>             issues must be|                            |
>>                      fixed|                            |
>> 
>>         --- Comment #1 from yonghong.zhu@intel.com ---
>>         Assign to Package owner.
>> 
>>         --
>>         You are receiving this mail because:
>>         You are the assignee for the bug.
>> 
>>       _______________________________________________
>>       edk2-devel mailing list
>>       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 <mailto:edk2-devel@lists.01.org>
> https://lists.01.org/mailman/listinfo/edk2-devel <https://lists.01.org/mailman/listinfo/edk2-devel>


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

* Re: [Bug 164] Add the build option "/D DISABLE_NEW_DEPRECATED_INTERFACES" in package DSC files
  2016-10-21 20:54           ` Andrew Fish
@ 2016-10-21 20:55             ` Andrew Fish
  0 siblings, 0 replies; 16+ messages in thread
From: Andrew Fish @ 2016-10-21 20:55 UTC (permalink / raw)
  To: Jordan Justen
  Cc: Ard Biesheuvel, edk2-devel-01, Leif Lindholm, Gao, Liming,
	Mike Kinney, Laszlo Ersek


> On Oct 21, 2016, at 1:54 PM, Andrew Fish <afish@apple.com> wrote:
> 
>> 
>> On Oct 21, 2016, at 1:39 PM, Jordan Justen <jordan.l.justen@intel.com> wrote:
>> 
>> On 2016-10-21 13:20:49, Andrew Fish wrote:
>>>    On Oct 21, 2016, at 12:58 PM, Jordan Justen <jordan.l.justen@intel.com>
>>>    wrote:
>>>    On 2016-10-21 12:37:21, Ard Biesheuvel wrote:
>>> 
>>>      I don't remember seeing any discussion regarding
>>>      DISABLE_NEW_DEPRECATED_INTERFACES on the list, so I am a bit surprised
>>>      seeing these bugs being filed and assigned.
>>> 
>>>    I agree.
>>> 
>>>    Also, the terminology seems confusing. 'new deprecated' seems like a
>>>    contradiction. I guess it means 'newly deprecated', but that seems
>>>    like a term that is quickly going to become obsolete. Soon there will
>>>    be old deprecated items that are disabled with this switch.
>>>    DISABLE_DEPRECATED_INTERFACES sounds better.
>>> 
>>>    But, shouldn't we have platforms opt-in to using the deprecated
>>>    interfaces rather than adding DISABLE_NEW_DEPRECATED_INTERFACES to the
>>>    build command line for every EDK II platform?
>>> 
>>>    Not using deprecated items should be the default for EDK II platforms.
>>>    If a platform has to opt-in to the deprecated content in their .dsc,
>>>    then it is obvious that they are relying on deprecated functionality.
>>> 
>>>    So, I guess I'd propose adding ENABLE_DEPRECATED_INTERFACES instead.
>>> 
>>>  Jordan,
>>>  I think it depends on your point of view. If you have a platform that
>>>  works and you update the edk2 revision you would expect it to still work.
>> 
>> I think this is what UDK is for. If you want to depend directly on EDK
>> II, then you'll see less stability.
>> 
> 
> Jordan,
> 
> Well there should be a published plan for a future UDK that this change is going to happen before we "break it" in master. Publishing the plan with the UDK does not count :). 
> 
>>>  Thus the option is to DISABLE_DEPRECATED_INTERFACES as that maintains
>>>  backward compatibility.
>> 
>> In order to support UDK releases, maybe ENABLE_UDK2014_INTERFACES would be
>> something to consider. Or ENABLE_UDK_INTERFACE=2014 so we can use <=.
>> 
>> But, I still think that EDK II platforms (as a goal) should represent
>> the best, cleanest examples of using EDK II. And, I think having every
>> platform accumulate cruft like CFLAGS to disable deprecated interfaces
>> works against that goal.
>> 
>> Another point. What about when we want to deprecate more interfaces?
>> Oh know, we better not break platforms that only specified
>> DISABLE_NEW_DEPRECATED_INTERFACES! Let's add
>> DISABLE_NEW_DEPRECATED_INTERFACES2! :)
>> 
> 
> I think you make a very good point. How about DISABLE_2014_DEPRECATE_INTERFACES. I think that version scales, and might actually encourage cleanup as it shows when the interface first got deprecated. 
> 

Sorry, DISABLE_2014_DEPRECATED_INTERFACES.

Thanks,

Andrew Fish

> Thanks,
> 
> Andrew Fish
> 
>> -Jordan
>> 
>>>  I think it makes total sense to turn on DISABLE_DEPRECATED_INTERFACES on
>>>  all the open source edk2 platform as soon as possible so all the open
>>>  source code is following current best practices.
>>>  Not to mention it would probably be a really good idea to give all the
>>>  downstream folks a long lead time about the plan of making a non backward
>>>  compatible change. 
>>>  Thanks,
>>>  Andrew Fish
>>> 
>>>    -Jordan
>>> 
>>>      Before making any such changes, I would like a strong commitment from
>>>      other package owners that deprecating an interface brings along with
>>>      it the responsibility to update all existing callers, otherwise
>>>      setting this define will only result in more breakage, and ARM has
>>>      seen its share of inadvertent breakage in the past when changes to
>>>      core code were made without taking other architectures into account.
>>> 
>>>      On 21 October 2016 at 02:21,  <bugzilla-daemon@bugzilla.tianocore.org>
>>>      wrote:
>>> 
>>>        https://bugzilla.tianocore.org/show_bug.cgi?id=164
>>> 
>>>        yonghong.zhu@intel.com changed:
>>> 
>>>                  What    |Removed                     |Added
>>>        ----------------------------------------------------------------------------
>>>                  Priority|Lowest                      |Normal
>>>                    Status|UNCONFIRMED                 |CONFIRMED
>>>                  Assignee|michael.d.kinney@intel.com
>>>         |ard.biesheuvel@linaro.org
>>>            Ever confirmed|0                           |1
>>>            Release(s) the|                            |EDK II Trunk
>>>            issues must be|                            |
>>>                     fixed|                            |
>>> 
>>>        --- Comment #1 from yonghong.zhu@intel.com ---
>>>        Assign to Package owner.
>>> 
>>>        --
>>>        You are receiving this mail because:
>>>        You are the assignee for the bug.
>>> 
>>>      _______________________________________________
>>>      edk2-devel mailing list
>>>      edk2-devel@lists.01.org
>>>      https://lists.01.org/mailman/listinfo/edk2-devel <https://lists.01.org/mailman/listinfo/edk2-devel> <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 <mailto:edk2-devel@lists.01.org> <mailto: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> <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 <mailto:edk2-devel@lists.01.org>
> https://lists.01.org/mailman/listinfo/edk2-devel <https://lists.01.org/mailman/listinfo/edk2-devel>


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

* Re: [Bug 164] Add the build option "/D DISABLE_NEW_DEPRECATED_INTERFACES" in package DSC files
  2016-10-21 20:40           ` Laszlo Ersek
@ 2016-10-21 20:57             ` Ard Biesheuvel
  0 siblings, 0 replies; 16+ messages in thread
From: Ard Biesheuvel @ 2016-10-21 20:57 UTC (permalink / raw)
  To: Laszlo Ersek
  Cc: Jordan Justen, edk2-devel-01, Kinney, Michael D, Leif Lindholm,
	afish@apple.com, Gao, Liming

On 21 October 2016 at 21:40, Laszlo Ersek <lersek@redhat.com> wrote:
> On 10/21/16 22:19, Ard Biesheuvel wrote:
>> On 21 October 2016 at 21:14, Laszlo Ersek <lersek@redhat.com> wrote:
>>> On 10/21/16 21:58, Jordan Justen wrote:
>>>> On 2016-10-21 12:37:21, Ard Biesheuvel wrote:
>>>>> I don't remember seeing any discussion regarding
>>>>> DISABLE_NEW_DEPRECATED_INTERFACES on the list, so I am a bit surprised
>>>>> seeing these bugs being filed and assigned.
>>>>>
>>>>
>>>> I agree.
>>>>
>>>> Also, the terminology seems confusing. 'new deprecated' seems like a
>>>> contradiction. I guess it means 'newly deprecated', but that seems
>>>> like a term that is quickly going to become obsolete. Soon there will
>>>> be old deprecated items that are disabled with this switch.
>>>> DISABLE_DEPRECATED_INTERFACES sounds better.
>>>>
>>>> But, shouldn't we have platforms opt-in to using the deprecated
>>>> interfaces rather than adding DISABLE_NEW_DEPRECATED_INTERFACES to the
>>>> build command line for every EDK II platform?
>>>>
>>>> Not using deprecated items should be the default for EDK II platforms.
>>>> If a platform has to opt-in to the deprecated content in their .dsc,
>>>> then it is obvious that they are relying on deprecated functionality.
>>>>
>>>> So, I guess I'd propose adding ENABLE_DEPRECATED_INTERFACES instead.
>>>
>>> I'm about to post a ~20-part series for OvmfPkg and ArmVirtPkg together
>>> that solves these BZs. :/ The DISABLE_NEW_DEPRECATED_INTERFACES feature
>>> test macro is already being used in three MdePkg library class header
>>> files (and a number of library instances), so I thought it was a done deal.
>>>
>>> I don't want to stifle the discussion of course, but at this point I
>>> think I will post the patches for review.
>>>
>>
>> Yes, please. Removing uses of deprecated interfaces is something we
>> should do anyway. What we shouldn't do is configure our platforms in
>> such a way that additional future deprecation automatically break the
>> build, unless we have a strong commitment from the other package
>> owners that they will ensure that this does not happen.
>>
>
> Well, my expectation is that the onus of keeping the tree working is on
> the patch submitter. Assume we adopt DISABLE_NEW_DEPRECATED_INTERFACES
> now (weaning ourselves off the functions that are *currently* disabled
> by the macro). Then whenever someone moves further functions under the
> scope of the macro -- thereby possibly breaking platform code --, it's
> going to be their responsibility to grep the tree for platform DSCs that
> enable DISABLE_NEW_DEPRECATED_INTERFACES, and either test-build those
> platforms reasonably extensively, or ask for help *in advance* (before
> committing the patches).
>
> For example, in the current work I'm about the post, I couldn't
> build-test everything (no RVCT over here, for example :)) Also I don't
> run Xen, so the Xen-related changes can't be functionally tested on my
> side -- but, I do ask for help with testing those. Same goes for the
> 32-bit ARM changes (which, it turns out, Michael and myself managed to
> fix separately, independently, and differently :))
>
> It's okay to modify code one can't build or test himself/herself, but
> then help should be asked for, and given too.
>
> TL;DR: the strong commitment you speak about is the default for any open
> source project, isn't it? ;)
>

Yes it is. But it should be made explicit.


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

* Re: [Bug 164] Add the build option "/D DISABLE_NEW_DEPRECATED_INTERFACES" in package DSC files
  2016-10-21 20:39         ` Jordan Justen
  2016-10-21 20:54           ` Andrew Fish
@ 2016-10-21 21:02           ` Laszlo Ersek
  2016-10-21 22:10             ` Jordan Justen
  1 sibling, 1 reply; 16+ messages in thread
From: Laszlo Ersek @ 2016-10-21 21:02 UTC (permalink / raw)
  To: Jordan Justen, Andrew Fish
  Cc: Ard Biesheuvel, edk2-devel-01, Mike Kinney, Leif Lindholm,
	Gao, Liming

On 10/21/16 22:39, Jordan Justen wrote:
> On 2016-10-21 13:20:49, Andrew Fish wrote:
>>      On Oct 21, 2016, at 12:58 PM, Jordan Justen <jordan.l.justen@intel.com>
>>      wrote:
>>      On 2016-10-21 12:37:21, Ard Biesheuvel wrote:
>>
>>        I don't remember seeing any discussion regarding
>>        DISABLE_NEW_DEPRECATED_INTERFACES on the list, so I am a bit surprised
>>        seeing these bugs being filed and assigned.
>>
>>      I agree.
>>
>>      Also, the terminology seems confusing. 'new deprecated' seems like a
>>      contradiction. I guess it means 'newly deprecated', but that seems
>>      like a term that is quickly going to become obsolete. Soon there will
>>      be old deprecated items that are disabled with this switch.
>>      DISABLE_DEPRECATED_INTERFACES sounds better.
>>
>>      But, shouldn't we have platforms opt-in to using the deprecated
>>      interfaces rather than adding DISABLE_NEW_DEPRECATED_INTERFACES to the
>>      build command line for every EDK II platform?
>>
>>      Not using deprecated items should be the default for EDK II platforms.
>>      If a platform has to opt-in to the deprecated content in their .dsc,
>>      then it is obvious that they are relying on deprecated functionality.
>>
>>      So, I guess I'd propose adding ENABLE_DEPRECATED_INTERFACES instead.
>>
>>    Jordan,
>>    I think it depends on your point of view. If you have a platform that
>>    works and you update the edk2 revision you would expect it to still work.
> 
> I think this is what UDK is for. If you want to depend directly on EDK
> II, then you'll see less stability.
> 
>>    Thus the option is to DISABLE_DEPRECATED_INTERFACES as that maintains
>>    backward compatibility.
> 
> In order to support UDK releases, maybe ENABLE_UDK2014_INTERFACES would be
> something to consider. Or ENABLE_UDK_INTERFACE=2014 so we can use <=.
> 
> But, I still think that EDK II platforms (as a goal) should represent
> the best, cleanest examples of using EDK II. And, I think having every
> platform accumulate cruft like CFLAGS to disable deprecated interfaces
> works against that goal.
> 
> Another point. What about when we want to deprecate more interfaces?
> Oh know, we better not break platforms that only specified
> DISABLE_NEW_DEPRECATED_INTERFACES! Let's add
> DISABLE_NEW_DEPRECATED_INTERFACES2! :)

Honestly, I imagined that DISABLE_NEW_DEPRECATED_INTERFACES would be
temporary in the edk2 tree. That is, it's a means so we can gradually
transition with all the in-tree stuff to a deprecationless code base.
Once that's done -- i.e., *all* platform DSCs within the edk2 tree
specify this feature test macro under their respective [BuildOptions]
sections --, then whatever the macro excises from the core packages can
be removed permanently, together with those platform [BuildOptions].

I think this should prevent the accumulation of cruft in edk2. Yes,
downstreams will have to catch up (or use UDK for a while longer). If
that's inconvenient, I have a solution: upstream your codebase, and then
the community will take care of keeping it in sync with the rest ;)

(This is the standard Linux suggestion BTW, not my idea.)

NB, we're not talking about protocols or PPIs (they're ABI); this is
about (statically linked) edk2-only libraries.

Thanks!
Laszlo

> 
> -Jordan
> 
>>    I think it makes total sense to turn on DISABLE_DEPRECATED_INTERFACES on
>>    all the open source edk2 platform as soon as possible so all the open
>>    source code is following current best practices.
>>    Not to mention it would probably be a really good idea to give all the
>>    downstream folks a long lead time about the plan of making a non backward
>>    compatible change. 
>>    Thanks,
>>    Andrew Fish
>>
>>      -Jordan
>>
>>        Before making any such changes, I would like a strong commitment from
>>        other package owners that deprecating an interface brings along with
>>        it the responsibility to update all existing callers, otherwise
>>        setting this define will only result in more breakage, and ARM has
>>        seen its share of inadvertent breakage in the past when changes to
>>        core code were made without taking other architectures into account.
>>
>>        On 21 October 2016 at 02:21,  <bugzilla-daemon@bugzilla.tianocore.org>
>>        wrote:
>>
>>          https://bugzilla.tianocore.org/show_bug.cgi?id=164
>>
>>          yonghong.zhu@intel.com changed:
>>
>>                    What    |Removed                     |Added
>>          ----------------------------------------------------------------------------
>>                    Priority|Lowest                      |Normal
>>                      Status|UNCONFIRMED                 |CONFIRMED
>>                    Assignee|michael.d.kinney@intel.com
>>           |ard.biesheuvel@linaro.org
>>              Ever confirmed|0                           |1
>>              Release(s) the|                            |EDK II Trunk
>>              issues must be|                            |
>>                       fixed|                            |
>>
>>          --- Comment #1 from yonghong.zhu@intel.com ---
>>          Assign to Package owner.
>>
>>          --
>>          You are receiving this mail because:
>>          You are the assignee for the bug.
>>
>>        _______________________________________________
>>        edk2-devel mailing list
>>        edk2-devel@lists.01.org
>>        https://lists.01.org/mailman/listinfo/edk2-devel



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

* Re: [Bug 164] Add the build option "/D DISABLE_NEW_DEPRECATED_INTERFACES" in package DSC files
  2016-10-21 21:02           ` Laszlo Ersek
@ 2016-10-21 22:10             ` Jordan Justen
  2016-10-21 22:31               ` Laszlo Ersek
  0 siblings, 1 reply; 16+ messages in thread
From: Jordan Justen @ 2016-10-21 22:10 UTC (permalink / raw)
  To: Laszlo Ersek, Andrew Fish
  Cc: Mike Kinney, edk2-devel-01, Gao, Liming, Leif Lindholm,
	Ard Biesheuvel

On 2016-10-21 14:02:44, Laszlo Ersek wrote:
> On 10/21/16 22:39, Jordan Justen wrote:
> > On 2016-10-21 13:20:49, Andrew Fish wrote:
> >>    Thus the option is to DISABLE_DEPRECATED_INTERFACES as that maintains
> >>    backward compatibility.
> > 
> > In order to support UDK releases, maybe ENABLE_UDK2014_INTERFACES would be
> > something to consider. Or ENABLE_UDK_INTERFACE=2014 so we can use <=.
> > 
> > But, I still think that EDK II platforms (as a goal) should represent
> > the best, cleanest examples of using EDK II. And, I think having every
> > platform accumulate cruft like CFLAGS to disable deprecated interfaces
> > works against that goal.
> > 
> > Another point. What about when we want to deprecate more interfaces?
> > Oh know, we better not break platforms that only specified
> > DISABLE_NEW_DEPRECATED_INTERFACES! Let's add
> > DISABLE_NEW_DEPRECATED_INTERFACES2! :)
> 
> Honestly, I imagined that DISABLE_NEW_DEPRECATED_INTERFACES would be
> temporary in the edk2 tree. That is, it's a means so we can gradually
> transition with all the in-tree stuff to a deprecationless code base.
> Once that's done -- i.e., *all* platform DSCs within the edk2 tree
> specify this feature test macro under their respective [BuildOptions]
> sections --, then whatever the macro excises from the core packages can
> be removed permanently, together with those platform [BuildOptions].
>

That could be reasonable, although I'd argue that we could flip it
around. Opt-in to the deprecated interfaces on all platforms, and then
start marking deprecated interfaces. Finally we could clean up
platforms and removed the override.

But ... I think DISABLE_NEW_DEPRECATED_INTERFACES was first added in:

commit bf4a3dbd4751b6411bdfc98bf3ac2c4f928bdfdf
Author: ydong10 <ydong10@6f19259b-4bc3-4df7-8a09-765794883524>
Date:   Wed May 30 07:36:00 2012 +0000

So, I guess it is not going to be removed anytime soon. :(

-Jordan

> I think this should prevent the accumulation of cruft in edk2. Yes,
> downstreams will have to catch up (or use UDK for a while longer). If
> that's inconvenient, I have a solution: upstream your codebase, and then
> the community will take care of keeping it in sync with the rest ;)
> 
> (This is the standard Linux suggestion BTW, not my idea.)
> 
> NB, we're not talking about protocols or PPIs (they're ABI); this is
> about (statically linked) edk2-only libraries.
> 
> Thanks!
> Laszlo


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

* Re: [Bug 164] Add the build option "/D DISABLE_NEW_DEPRECATED_INTERFACES" in package DSC files
  2016-10-21 22:10             ` Jordan Justen
@ 2016-10-21 22:31               ` Laszlo Ersek
  2016-10-21 23:13                 ` Yao, Jiewen
  0 siblings, 1 reply; 16+ messages in thread
From: Laszlo Ersek @ 2016-10-21 22:31 UTC (permalink / raw)
  To: Jordan Justen, Andrew Fish
  Cc: Mike Kinney, edk2-devel-01, Gao, Liming, Leif Lindholm,
	Ard Biesheuvel

On 10/22/16 00:10, Jordan Justen wrote:
> On 2016-10-21 14:02:44, Laszlo Ersek wrote:

>> Honestly, I imagined that DISABLE_NEW_DEPRECATED_INTERFACES would be
>> temporary in the edk2 tree. That is, it's a means so we can gradually
>> transition with all the in-tree stuff to a deprecationless code base.
>> Once that's done -- i.e., *all* platform DSCs within the edk2 tree
>> specify this feature test macro under their respective [BuildOptions]
>> sections --, then whatever the macro excises from the core packages can
>> be removed permanently, together with those platform [BuildOptions].
>>
> 
> That could be reasonable, although I'd argue that we could flip it
> around. Opt-in to the deprecated interfaces on all platforms, and then
> start marking deprecated interfaces. Finally we could clean up
> platforms and removed the override.

That's a valid idea, IMO.

> But ... I think DISABLE_NEW_DEPRECATED_INTERFACES was first added in:
> 
> commit bf4a3dbd4751b6411bdfc98bf3ac2c4f928bdfdf
> Author: ydong10 <ydong10@6f19259b-4bc3-4df7-8a09-765794883524>
> Date:   Wed May 30 07:36:00 2012 +0000
> 
> So, I guess it is not going to be removed anytime soon. :(

I believe we just need to make progress with the individual platforms
(and their dependencies from other Pkgs). It's not a lot of fun, but the
BZs exist (well, they can be filed) now, and then we can address them...

I mean, I didn't care (or, really, know) about
DISABLE_NEW_DEPRECATED_INTERFACES until the ArmVirtPkg / OvmfPkg BZs got
filed. Bugzilla is great. I like the attention that it gets, from others
and from myself.

Thanks
Laszlo


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

* Re: [Bug 164] Add the build option "/D DISABLE_NEW_DEPRECATED_INTERFACES" in package DSC files
  2016-10-21 22:31               ` Laszlo Ersek
@ 2016-10-21 23:13                 ` Yao, Jiewen
  2016-10-23 14:28                   ` Mudusuru, Giri P
  0 siblings, 1 reply; 16+ messages in thread
From: Yao, Jiewen @ 2016-10-21 23:13 UTC (permalink / raw)
  To: Laszlo Ersek, Justen, Jordan L, Andrew Fish
  Cc: Kinney, Michael D, edk2-devel-01, Leif Lindholm, Gao, Liming,
	Ard Biesheuvel

I remember our deprecation process is:

1)      Core defines DISABLE_NEW_DEPRECATED_INTERFACES and puts a deprecated content in it. (Platform does not use DISABLE_NEW_DEPRECATED_INTERFACES and deprecated function can still be used at this moment. But we strongly recommend a platform doing clean up at same time.)

2)      Platform defines DISABLE_NEW_DEPRECATED_INTERFACES and deprecated function cannot be used after the clean up work.

3)      Core removes the deprecated content and DISABLE_NEW_DEPRECATED_INTERFACES, if we can make sure no platform using it.

4)      Platform may remove DISABLE_NEW_DEPRECATED_INTERFACES.

We do not want to remove a function directly, to break lots of platforms. We just want to give a buffer to let platform do code cleanup.

Thank you
Yao Jiewen

From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Laszlo Ersek
Sent: Saturday, October 22, 2016 6:31 AM
To: Justen, Jordan L <jordan.l.justen@intel.com>; Andrew Fish <afish@apple.com>
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; edk2-devel-01 <edk2-devel@ml01.01.org>; Leif Lindholm <leif.lindholm@linaro.org>; Gao, Liming <liming.gao@intel.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>
Subject: Re: [edk2] [Bug 164] Add the build option "/D DISABLE_NEW_DEPRECATED_INTERFACES" in package DSC files

On 10/22/16 00:10, Jordan Justen wrote:
> On 2016-10-21 14:02:44, Laszlo Ersek wrote:

>> Honestly, I imagined that DISABLE_NEW_DEPRECATED_INTERFACES would be
>> temporary in the edk2 tree. That is, it's a means so we can gradually
>> transition with all the in-tree stuff to a deprecationless code base.
>> Once that's done -- i.e., *all* platform DSCs within the edk2 tree
>> specify this feature test macro under their respective [BuildOptions]
>> sections --, then whatever the macro excises from the core packages can
>> be removed permanently, together with those platform [BuildOptions].
>>
>
> That could be reasonable, although I'd argue that we could flip it
> around. Opt-in to the deprecated interfaces on all platforms, and then
> start marking deprecated interfaces. Finally we could clean up
> platforms and removed the override.

That's a valid idea, IMO.

> But ... I think DISABLE_NEW_DEPRECATED_INTERFACES was first added in:
>
> commit bf4a3dbd4751b6411bdfc98bf3ac2c4f928bdfdf
> Author: ydong10 <ydong10@6f19259b-4bc3-4df7-8a09-765794883524>
> Date:   Wed May 30 07:36:00 2012 +0000
>
> So, I guess it is not going to be removed anytime soon. :(

I believe we just need to make progress with the individual platforms
(and their dependencies from other Pkgs). It's not a lot of fun, but the
BZs exist (well, they can be filed) now, and then we can address them...

I mean, I didn't care (or, really, know) about
DISABLE_NEW_DEPRECATED_INTERFACES until the ArmVirtPkg / OvmfPkg BZs got
filed. Bugzilla is great. I like the attention that it gets, from others
and from myself.

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


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

* Re: [Bug 164] Add the build option "/D DISABLE_NEW_DEPRECATED_INTERFACES" in package DSC files
  2016-10-21 23:13                 ` Yao, Jiewen
@ 2016-10-23 14:28                   ` Mudusuru, Giri P
  0 siblings, 0 replies; 16+ messages in thread
From: Mudusuru, Giri P @ 2016-10-23 14:28 UTC (permalink / raw)
  To: Yao, Jiewen, Laszlo Ersek, Justen, Jordan L, Andrew Fish
  Cc: Kinney, Michael D, edk2-devel-01, Gao, Liming, Leif Lindholm,
	Ard Biesheuvel

A combination of Jordon's and Andrews proposal would be better so we have one flag and also scalable. Level of deprecated interfaces can be controlled by each platform. EDK2 master platforms should not define this flag to eliminate the usage of deprecated interfaces while UDK2015 can define and set the value to 2015 for compatibility. 

ENABLE_UDK_DEPRECATED_INTERFACES=2014 or 2015 or 2017 (UDK versions)

Thanks,
-Giri

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Yao,
> Jiewen
> Sent: Friday, October 21, 2016 4:13 PM
> To: Laszlo Ersek <lersek@redhat.com>; Justen, Jordan L
> <jordan.l.justen@intel.com>; Andrew Fish <afish@apple.com>
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; edk2-devel-01 <edk2-
> devel@ml01.01.org>; Gao, Liming <liming.gao@intel.com>; Leif Lindholm
> <leif.lindholm@linaro.org>; Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Subject: Re: [edk2] [Bug 164] Add the build option "/D
> DISABLE_NEW_DEPRECATED_INTERFACES" in package DSC files
> 
> I remember our deprecation process is:
> 
> 1)      Core defines DISABLE_NEW_DEPRECATED_INTERFACES and puts a
> deprecated content in it. (Platform does not use
> DISABLE_NEW_DEPRECATED_INTERFACES and deprecated function can still be
> used at this moment. But we strongly recommend a platform doing clean up at
> same time.)
> 
> 2)      Platform defines DISABLE_NEW_DEPRECATED_INTERFACES and
> deprecated function cannot be used after the clean up work.
> 
> 3)      Core removes the deprecated content and
> DISABLE_NEW_DEPRECATED_INTERFACES, if we can make sure no platform
> using it.
> 
> 4)      Platform may remove DISABLE_NEW_DEPRECATED_INTERFACES.
> 
> We do not want to remove a function directly, to break lots of platforms. We
> just want to give a buffer to let platform do code cleanup.
> 
> Thank you
> Yao Jiewen
> 
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Laszlo
> Ersek
> Sent: Saturday, October 22, 2016 6:31 AM
> To: Justen, Jordan L <jordan.l.justen@intel.com>; Andrew Fish
> <afish@apple.com>
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; edk2-devel-01 <edk2-
> devel@ml01.01.org>; Leif Lindholm <leif.lindholm@linaro.org>; Gao, Liming
> <liming.gao@intel.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Subject: Re: [edk2] [Bug 164] Add the build option "/D
> DISABLE_NEW_DEPRECATED_INTERFACES" in package DSC files
> 
> On 10/22/16 00:10, Jordan Justen wrote:
> > On 2016-10-21 14:02:44, Laszlo Ersek wrote:
> 
> >> Honestly, I imagined that DISABLE_NEW_DEPRECATED_INTERFACES would
> be
> >> temporary in the edk2 tree. That is, it's a means so we can gradually
> >> transition with all the in-tree stuff to a deprecationless code base.
> >> Once that's done -- i.e., *all* platform DSCs within the edk2 tree
> >> specify this feature test macro under their respective [BuildOptions]
> >> sections --, then whatever the macro excises from the core packages can
> >> be removed permanently, together with those platform [BuildOptions].
> >>
> >
> > That could be reasonable, although I'd argue that we could flip it
> > around. Opt-in to the deprecated interfaces on all platforms, and then
> > start marking deprecated interfaces. Finally we could clean up
> > platforms and removed the override.
> 
> That's a valid idea, IMO.
> 
> > But ... I think DISABLE_NEW_DEPRECATED_INTERFACES was first added in:
> >
> > commit bf4a3dbd4751b6411bdfc98bf3ac2c4f928bdfdf
> > Author: ydong10 <ydong10@6f19259b-4bc3-4df7-8a09-765794883524>
> > Date:   Wed May 30 07:36:00 2012 +0000
> >
> > So, I guess it is not going to be removed anytime soon. :(
> 
> I believe we just need to make progress with the individual platforms
> (and their dependencies from other Pkgs). It's not a lot of fun, but the
> BZs exist (well, they can be filed) now, and then we can address them...
> 
> I mean, I didn't care (or, really, know) about
> DISABLE_NEW_DEPRECATED_INTERFACES until the ArmVirtPkg / OvmfPkg BZs
> got
> filed. Bugzilla is great. I like the attention that it gets, from others
> and from myself.
> 
> 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
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


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

end of thread, other threads:[~2016-10-23 14:28 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <bug-164-63@https.bugzilla.tianocore.org/>
     [not found] ` <bug-164-63-L8k0GFC2io@https.bugzilla.tianocore.org/>
2016-10-21 19:37   ` [Bug 164] Add the build option "/D DISABLE_NEW_DEPRECATED_INTERFACES" in package DSC files Ard Biesheuvel
2016-10-21 19:41     ` Michael Zimmermann
2016-10-21 19:58     ` Jordan Justen
2016-10-21 20:14       ` Laszlo Ersek
2016-10-21 20:19         ` Ard Biesheuvel
2016-10-21 20:40           ` Laszlo Ersek
2016-10-21 20:57             ` Ard Biesheuvel
2016-10-21 20:20       ` Andrew Fish
2016-10-21 20:39         ` Jordan Justen
2016-10-21 20:54           ` Andrew Fish
2016-10-21 20:55             ` Andrew Fish
2016-10-21 21:02           ` Laszlo Ersek
2016-10-21 22:10             ` Jordan Justen
2016-10-21 22:31               ` Laszlo Ersek
2016-10-21 23:13                 ` Yao, Jiewen
2016-10-23 14:28                   ` Mudusuru, Giri P

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