[v2,1/2] eal/windows: fix symbol export

Message ID 20201016102711.11926-1-david.marchand@redhat.com (mailing list archive)
State Accepted, archived
Delegated to: David Marchand
Headers
Series [v2,1/2] eal/windows: fix symbol export |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

David Marchand Oct. 16, 2020, 10:27 a.m. UTC
  The incriminated commit forgot to clean the Windows export file.

Fixes: 3cd73a1a1c4d ("eal: simplify exit functions")

Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Thomas Monjalon <thomas@monjalon.net>
---
 lib/librte_eal/rte_eal_exports.def | 1 -
 1 file changed, 1 deletion(-)
  

Comments

Ray Kinsella Oct. 16, 2020, 11:16 a.m. UTC | #1
The windows exports and the map files, feels like duplication of effort. 
Could we massage one into the other during the build?

On 16/10/2020 11:27, David Marchand wrote:
> The incriminated commit forgot to clean the Windows export file.
> 
> Fixes: 3cd73a1a1c4d ("eal: simplify exit functions")
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> Acked-by: Thomas Monjalon <thomas@monjalon.net>
> ---
>  lib/librte_eal/rte_eal_exports.def | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/lib/librte_eal/rte_eal_exports.def b/lib/librte_eal/rte_eal_exports.def
> index 16f8e33874..975acb8ffe 100644
> --- a/lib/librte_eal/rte_eal_exports.def
> +++ b/lib/librte_eal/rte_eal_exports.def
> @@ -32,7 +32,6 @@ EXPORTS
>  	rte_devargs_remove
>  	rte_devargs_type_count
>  	rte_dump_physmem_layout
> -	rte_dump_registers
>  	rte_dump_stack
>  	rte_dump_tailq
>  	rte_eal_alarm_cancel
> 

Acked-by: Ray Kinsella <mdr@ashroe.eu>
  
David Marchand Oct. 16, 2020, 11:22 a.m. UTC | #2
On Fri, Oct 16, 2020 at 1:16 PM Kinsella, Ray <mdr@ashroe.eu> wrote:
>
>
> The windows exports and the map files, feels like duplication of effort.
> Could we massage one into the other during the build?

That's what is done with map-to-win.py, unless we have one exception
when the full library is not ready, like EAL.

https://git.dpdk.org/dpdk/tree/lib/meson.build#n152
https://git.dpdk.org/dpdk/tree/buildtools/map_to_win.py#n27
  
Ray Kinsella Oct. 16, 2020, 11:23 a.m. UTC | #3
ah ... perfect.

Ray K

On 16/10/2020 12:22, David Marchand wrote:
> On Fri, Oct 16, 2020 at 1:16 PM Kinsella, Ray <mdr@ashroe.eu> wrote:
>>
>>
>> The windows exports and the map files, feels like duplication of effort.
>> Could we massage one into the other during the build?
> 
> That's what is done with map-to-win.py, unless we have one exception
> when the full library is not ready, like EAL.
> 
> https://git.dpdk.org/dpdk/tree/lib/meson.build#n152
> https://git.dpdk.org/dpdk/tree/buildtools/map_to_win.py#n27
> 
>
  
Ray Kinsella Oct. 16, 2020, 3:48 p.m. UTC | #4
On 16/10/2020 12:22, David Marchand wrote:
> On Fri, Oct 16, 2020 at 1:16 PM Kinsella, Ray <mdr@ashroe.eu> wrote:
>>
>>
>> The windows exports and the map files, feels like duplication of effort.
>> Could we massage one into the other during the build?
> 
> That's what is done with map-to-win.py, unless we have one exception
> when the full library is not ready, like EAL.
> 
> https://git.dpdk.org/dpdk/tree/lib/meson.build#n152
> https://git.dpdk.org/dpdk/tree/buildtools/map_to_win.py#n27
> 

Thinking about this again - future work might be to check if a .def exists.
And then to warn in checkpatch if one is modified without the other?

Ray K
  
Bruce Richardson Oct. 16, 2020, 3:52 p.m. UTC | #5
On Fri, Oct 16, 2020 at 04:48:59PM +0100, Kinsella, Ray wrote:
> 
> On 16/10/2020 12:22, David Marchand wrote:
> > On Fri, Oct 16, 2020 at 1:16 PM Kinsella, Ray <mdr@ashroe.eu> wrote:
> >>
> >>
> >> The windows exports and the map files, feels like duplication of effort.
> >> Could we massage one into the other during the build?
> > 
> > That's what is done with map-to-win.py, unless we have one exception
> > when the full library is not ready, like EAL.
> > 
> > https://git.dpdk.org/dpdk/tree/lib/meson.build#n152
> > https://git.dpdk.org/dpdk/tree/buildtools/map_to_win.py#n27
> > 
> 
> Thinking about this again - future work might be to check if a .def exists.
> And then to warn in checkpatch if one is modified without the other?
> 
Possibly, but right now there is only the one .def override file, for EAL,
and a better use of effort is to close the gap between windows and other
versions so that it can be removed completely.

$ find lib/ drivers/ -name *.def
lib/librte_eal/rte_eal_exports.def

/Bruce
  
Thomas Monjalon Oct. 16, 2020, 4:02 p.m. UTC | #6
16/10/2020 17:48, Kinsella, Ray:
> On 16/10/2020 12:22, David Marchand wrote:
> > On Fri, Oct 16, 2020 at 1:16 PM Kinsella, Ray <mdr@ashroe.eu> wrote:
> >>
> >> The windows exports and the map files, feels like duplication of effort.
> >> Could we massage one into the other during the build?
> > 
> > That's what is done with map-to-win.py, unless we have one exception
> > when the full library is not ready, like EAL.
> > 
> > https://git.dpdk.org/dpdk/tree/lib/meson.build#n152
> > https://git.dpdk.org/dpdk/tree/buildtools/map_to_win.py#n27
> 
> Thinking about this again - future work might be to check if a .def exists.
> And then to warn in checkpatch if one is modified without the other?

It would not have avoided this miss,
because the symbol has been added in the .def
after I've sent my patch removing the function.
What missed was a check to run before pushing,
and this is what David did in the patch 2 of this series.
  

Patch

diff --git a/lib/librte_eal/rte_eal_exports.def b/lib/librte_eal/rte_eal_exports.def
index 16f8e33874..975acb8ffe 100644
--- a/lib/librte_eal/rte_eal_exports.def
+++ b/lib/librte_eal/rte_eal_exports.def
@@ -32,7 +32,6 @@  EXPORTS
 	rte_devargs_remove
 	rte_devargs_type_count
 	rte_dump_physmem_layout
-	rte_dump_registers
 	rte_dump_stack
 	rte_dump_tailq
 	rte_eal_alarm_cancel