From: Giuliano Procida <gprocida@google.com>
To: Dodji Seketeli <dodji@seketeli.org>
Cc: libabigail@sourceware.org, kernel-team@android.com,
"Matthias Männich" <maennich@google.com>
Subject: Re: [PATCH v3 1/1] Fix has_net_changes for --leaf-changes-only mode
Date: Fri, 17 Jul 2020 17:34:59 +0100 [thread overview]
Message-ID: <CAGvU0H=gSM_2qnf9TtNcLrRvp0_N_oMDcexFVLPRuRtSrsQMgQ@mail.gmail.com> (raw)
In-Reply-To: <87a6zyi6vq.fsf@seketeli.org>
Hi Dodji
This looks good to me and I concur it's more in keeping with the rest
of the code.
A couple of nits inline.
On Fri, 17 Jul 2020 at 14:54, Dodji Seketeli <dodji@seketeli.org> wrote:
>
> Hello Giuliano,
>
> Giuliano Procida <gprocida@google.com> a écrit:
>
> [...]
>
> > diff --git a/src/abg-comparison.cc b/src/abg-comparison.cc
> > index c1d86a7f..e4b86426 100644
> > --- a/src/abg-comparison.cc
> > +++ b/src/abg-comparison.cc
> > @@ -10847,21 +10847,27 @@ corpus_diff::has_net_changes() const
> > const diff_stats& stats = const_cast<corpus_diff*>(this)->
> > apply_filters_and_suppressions_before_reporting();
> >
> > + // Logic here should match emit_diff_stats.
> > + // TODO: Possibly suppress things that won't be shown there.
> > + bool leaf = context()->show_leaf_changes_only();
>
> I think that right here, we shouldn't be making assumptions about the
> kind of reporter (leaf or default) we are using. This is a relic from
> the ancient time predating the support of multiple reporters. I (maybe
> too) progressively started to delegate reporter-specific behaviour to
> the reporters, especially with how corpus_diff::has_net_changes is
> implemented.
>
> I think we should move further into that direction, to keep a more
> maintainable code base. In practise, that means that this comparison
> engine should remain as uncoupled from the reporters as possible .
>
> So I have amended your patch a little bit to achieve that, in the same
> spirit as what has been done for corpus_diff::has_net_changes.
>
> Please find below the amended patch I am proposing to apply instead.
> The tests are all the same and pass distcheck.
>
> From 76e53729f426e620268ebb43746476684112023d Mon Sep 17 00:00:00 2001
> From: Giuliano Procida <gprocida@google.com>
> Date: Fri, 17 Jul 2020 14:01:01 +0200
> Subject: [PATCH] Fix corpus_diff::has_net_changes for --leaf-changes-only mode
>
> This function was not aware of --leaf-changes-only mode.
>
> - Stats counters for changed variables and types have
> different names in the different modes.
> - Net leaf type changes were not included in leaf mode.
>
> For some inputs, this resulted in abidiff producing an empty report
> but returning a non-zero exit status in --leaf-changes-only mode.
>
> For other inputs the combination of both issues still resulted in the
> correct return code. This included the following test-abidiff-exit
> test cases:
>
> - test-leaf-peeling
> - test-leaf2
> - test-no-stray-comma
>
> This patch makes corpus_diff::has_net_changes mirror emit_diff_stats,
> modulo flags like --non-reachable-types which if absent can still
> result in discrepancies between output and return code.
>
> To achieve this in a more maintainable way, the patch introduces a new interface
> reporter_base::diff_has_net_changes. That interface is implemented by
> all current reporters. Each reporter focuses on its own
> particularities to provide the required behavious. Then
> corpus_diff:has_net_changes just has to invoke
> reporter_base::diff_has_net_changes on the reporter that is currently
> in used.
>
> The tests below verify that the exit code is zero when all the changes
> between the test files are suppressed.
>
> * include/abg-reporter.h ({reporter_base, default_reporter,
> leaf_reporter}::diff_has_net_changes): Add new virtual function.
> This breaks binary compatibility but should conserve source
> compatibility.
> * src/abg-default-reporter.cc
> (default_reporter::diff_has_net_changes): Define new member
> function.
> * src/abg-leaf-reporter.cc (leaf_reporter::diff_has_net_changes):
> Likewise.
> * src/abg-comparison.cc (corpus_diff::has_net_changes): Invoke
> reporter_base::diff_has_net_changes on the current reporter,
> rather than trying to handle all the different kinds of reporters
> here.
> (corpus_diff::priv::apply_filters_and_compute_diff_stats): Add a
> TODO to possibly delegate the implementation of this function to
> the reporters.
> * tests/data/Makefile.am: Add new test case files.
> * tests/data/test-abidiff-exit/test-net-change-report0.txt:
> Normal mode, nothing suppressed.
> * tests/data/test-abidiff-exit/test-net-change-report1.txt:
> Normal mode, everything suppressed.
> * tests/data/test-abidiff-exit/test-net-change-report2.txt:
> Leaf mode, nothing suppressed.
> * tests/data/test-abidiff-exit/test-net-change-report3.txt:
> Leaf mode, everything suppressions.
> * tests/data/test-abidiff-exit/test-net-change-v0.c: Test file
> * tests/data/test-abidiff-exit/test-net-change-v0.o: Test file
> * tests/data/test-abidiff-exit/test-net-change-v1.c: Test file
> * tests/data/test-abidiff-exit/test-net-change-v1.o: Test file
> * tests/data/test-abidiff-exit/test-net-change.abignore: This
> suppresses changes for all variables, functions and types in
> the test files, except for the 'victim' function.
> * tests/test-abidiff-exit.cc: Run new test cases.
>
> Reviewed-by: Matthias Maennich <maennich@google.com>
> Signed-off-by: Giuliano Procida <gprocida@google.com>
> Signed-off-by: Dodji Seketeli <dodji@redhat.com>
> ---
> include/abg-reporter.h | 6 +++
> src/abg-comparison.cc | 24 ++----------
> src/abg-default-reporter.cc | 35 +++++++++++++++++
> src/abg-leaf-reporter.cc | 36 +++++++++++++++++
> tests/data/Makefile.am | 9 +++++
> .../test-abidiff-exit/test-net-change-report0.txt | 43 +++++++++++++++++++++
> .../test-abidiff-exit/test-net-change-report1.txt | 3 ++
> .../test-abidiff-exit/test-net-change-report2.txt | 42 ++++++++++++++++++++
> .../test-abidiff-exit/test-net-change-report3.txt | 5 +++
> tests/data/test-abidiff-exit/test-net-change-v0.c | 14 +++++++
> tests/data/test-abidiff-exit/test-net-change-v0.o | Bin 0 -> 3512 bytes
> tests/data/test-abidiff-exit/test-net-change-v1.c | 14 +++++++
> tests/data/test-abidiff-exit/test-net-change-v1.o | Bin 0 -> 3552 bytes
> .../test-abidiff-exit/test-net-change.abignore | 8 ++++
> tests/test-abidiff-exit.cc | 38 ++++++++++++++++++
> 15 files changed, 257 insertions(+), 20 deletions(-)
> create mode 100644 tests/data/test-abidiff-exit/test-net-change-report0.txt
> create mode 100644 tests/data/test-abidiff-exit/test-net-change-report1.txt
> create mode 100644 tests/data/test-abidiff-exit/test-net-change-report2.txt
> create mode 100644 tests/data/test-abidiff-exit/test-net-change-report3.txt
> create mode 100644 tests/data/test-abidiff-exit/test-net-change-v0.c
> create mode 100644 tests/data/test-abidiff-exit/test-net-change-v0.o
> create mode 100644 tests/data/test-abidiff-exit/test-net-change-v1.c
> create mode 100644 tests/data/test-abidiff-exit/test-net-change-v1.o
> create mode 100644 tests/data/test-abidiff-exit/test-net-change.abignore
>
> diff --git a/include/abg-reporter.h b/include/abg-reporter.h
> index e0d9e66..bf113f0 100644
> --- a/include/abg-reporter.h
> +++ b/include/abg-reporter.h
> @@ -73,6 +73,8 @@ public:
>
> virtual bool diff_to_be_reported(const diff *d) const;
>
> + virtual bool diff_has_net_changes(const corpus_diff *d) const = 0;
> +
> virtual void
> report(const type_decl_diff& d, std::ostream& out,
> const std::string& indent = "") const = 0;
> @@ -162,6 +164,8 @@ class default_reporter : public reporter_base
> {
> public:
>
> + virtual bool diff_has_net_changes(const corpus_diff *d) const;
> +
> virtual void
> report(const type_decl_diff& d, std::ostream& out,
> const std::string& indent = "") const;
> @@ -266,6 +270,8 @@ public:
>
> virtual bool diff_to_be_reported(const diff *d) const;
>
> + virtual bool diff_has_net_changes(const corpus_diff *d) const;
> +
> void
> report_changes_from_diff_maps(const diff_maps&, std::ostream& out,
> const std::string& indent) const;
> diff --git a/src/abg-comparison.cc b/src/abg-comparison.cc
> index b6f065f..a9ef161 100644
> --- a/src/abg-comparison.cc
> +++ b/src/abg-comparison.cc
> @@ -10063,6 +10063,9 @@ corpus_diff::priv::apply_filters_and_compute_diff_stats(diff_stats& stat)
> /// Emit the summary of the functions & variables that got
> /// removed/changed/added.
> ///
> +/// TODO: This should be handled by the reporters, just like what is
> +/// done for reporter_base::diff_to_be_reported.
> +///
> /// @param out the output stream to emit the stats to.
> ///
> /// @param indent the indentation string to use in the summary.
> @@ -10848,26 +10851,7 @@ corpus_diff::has_net_subtype_changes() const
> /// carries subtype changes that are deemed incompatible ABI changes.
> bool
> corpus_diff::has_net_changes() const
> -{
> - const diff_stats& stats = const_cast<corpus_diff*>(this)->
> - apply_filters_and_suppressions_before_reporting();
> -
> - return (architecture_changed()
> - || soname_changed()
> - || stats.net_num_func_changed()
> - || stats.net_num_vars_changed()
> - || stats.net_num_func_added()
> - || stats.net_num_added_func_syms()
> - || stats.net_num_func_removed()
> - || stats.net_num_removed_func_syms()
> - || stats.net_num_vars_added()
> - || stats.net_num_added_var_syms()
> - || stats.net_num_vars_removed()
> - || stats.net_num_removed_var_syms()
> - || stats.net_num_added_unreachable_types()
> - || stats.net_num_removed_unreachable_types()
> - || stats.net_num_changed_unreachable_types());
> -}
> +{return context()->get_reporter()->diff_has_net_changes(this);}
>
> /// Apply the different filters that are registered to be applied to
> /// the diff tree; that includes the categorization filters. Also,
> diff --git a/src/abg-default-reporter.cc b/src/abg-default-reporter.cc
> index 2948e15..24a05fa 100644
> --- a/src/abg-default-reporter.cc
> +++ b/src/abg-default-reporter.cc
> @@ -35,6 +35,41 @@ namespace abigail
> namespace comparison
> {
>
> +/// Test if a given instance of @ref corpus_diff carries changes whose
> +/// reports are not suppressed by any suppression specification. In
> +/// effect, these are deemed incompatible ABI changes.
> +///
> +/// @param d the @ref corpus_diff to consider
> +///
> +/// @return true iff @p d carries subtype changes that are deemed
> +/// incompatible ABI changes.
> +bool
> +default_reporter::diff_has_net_changes(const corpus_diff *d) const
> +{
> + if (!d)
> + return false;
> +
> + const corpus_diff::diff_stats& stats = const_cast<corpus_diff*>(d)->
> + apply_filters_and_suppressions_before_reporting();
> +
> + // Logic here should match emit_diff_stats.
> + return (d->architecture_changed()
I have a preference for not doing return (expression). It's not an
issue if you find the extra parentheses help.
> + ||d->soname_changed()
Missing a space before the "d".
> + || stats.net_num_func_removed()
> + || stats.net_num_func_changed()
> + || stats.net_num_func_added()
> + || stats.net_num_vars_removed()
> + || stats.net_num_vars_changed()
> + || stats.net_num_vars_added()
> + || stats.net_num_removed_unreachable_types()
> + || stats.net_num_changed_unreachable_types()
> + || stats.net_num_added_unreachable_types()
> + || stats.net_num_removed_func_syms()
> + || stats.net_num_added_func_syms()
> + || stats.net_num_removed_var_syms()
> + || stats.net_num_added_var_syms());
> +}
> +
> /// Ouputs a report of the differences between of the two type_decl
> /// involved in the @ref type_decl_diff.
> ///
> diff --git a/src/abg-leaf-reporter.cc b/src/abg-leaf-reporter.cc
> index 783ffbc..0875c96 100644
> --- a/src/abg-leaf-reporter.cc
> +++ b/src/abg-leaf-reporter.cc
> @@ -45,6 +45,42 @@ bool
> leaf_reporter::diff_to_be_reported(const diff *d) const
> {return d && d->to_be_reported() && d->has_local_changes();}
>
> +/// Test if a given instance of @ref corpus_diff carries changes whose
> +/// reports are not suppressed by any suppression specification. In
> +/// effect, these are deemed incompatible ABI changes.
> +///
> +/// @param d the @ref corpus_diff to consider
> +///
> +/// @return true iff @p d carries subtype changes that are deemed
> +/// incompatible ABI changes.
> +bool
> +leaf_reporter::diff_has_net_changes(const corpus_diff *d) const
> +{
> + if (!d)
> + return false;
> +
> + const corpus_diff::diff_stats& stats = const_cast<corpus_diff*>(d)->
> + apply_filters_and_suppressions_before_reporting();
> +
> + // Logic here should match emit_diff_stats.
> + return (d->architecture_changed()
Could also make this plain return expression.
> + || d->soname_changed()
> + || stats.net_num_func_removed()
> + || stats.net_num_leaf_type_changes()
> + || stats.net_num_leaf_func_changes()
> + || stats.net_num_func_added()
> + || stats.net_num_vars_removed()
> + || stats.net_num_leaf_var_changes()
> + || stats.net_num_vars_added()
> + || stats.net_num_removed_unreachable_types()
> + || stats.net_num_changed_unreachable_types()
> + || stats.net_num_added_unreachable_types()
> + || stats.net_num_removed_func_syms()
> + || stats.net_num_added_func_syms()
> + || stats.net_num_removed_var_syms()
> + || stats.net_num_added_var_syms());
> +}
> +
> /// Report the changes carried by the diffs contained in an instance
> /// of @ref string_diff_ptr_map.
> ///
> diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
> index 5da9ef9..ea94f0b 100644
> --- a/tests/data/Makefile.am
> +++ b/tests/data/Makefile.am
> @@ -165,6 +165,15 @@ test-abidiff-exit/test-decl-enum-v1.o \
> test-abidiff-exit/test-decl-enum-report.txt \
> test-abidiff-exit/test-decl-enum-report-2.txt \
> test-abidiff-exit/test-decl-enum-report-3.txt \
> +test-abidiff-exit/test-net-change.abignore \
> +test-abidiff-exit/test-net-change-v0.c \
> +test-abidiff-exit/test-net-change-v0.o \
> +test-abidiff-exit/test-net-change-v1.c \
> +test-abidiff-exit/test-net-change-v1.o \
> +test-abidiff-exit/test-net-change-report0.txt \
> +test-abidiff-exit/test-net-change-report1.txt \
> +test-abidiff-exit/test-net-change-report2.txt \
> +test-abidiff-exit/test-net-change-report3.txt \
> \
> test-diff-dwarf/test0-v0.cc \
> test-diff-dwarf/test0-v0.o \
> diff --git a/tests/data/test-abidiff-exit/test-net-change-report0.txt b/tests/data/test-abidiff-exit/test-net-change-report0.txt
> new file mode 100644
> index 0000000..66712b0
> --- /dev/null
> +++ b/tests/data/test-abidiff-exit/test-net-change-report0.txt
> @@ -0,0 +1,43 @@
> +Functions changes summary: 1 Removed, 2 Changed, 1 Added functions
> +Variables changes summary: 1 Removed, 1 Changed, 1 Added variables
> +
> +1 Removed function:
> +
> + [D] 'function int fun_removed()' {fun_removed}
> +
> +1 Added function:
> +
> + [A] 'function long int fun_added()' {fun_added}
> +
> +2 functions with some indirect sub-type change:
> +
> + [C] 'function int fun_changed()' has some indirect sub-type changes:
> + return type changed:
> + type name changed from 'int' to 'long int'
> + type size changed from 32 to 64 (in bits)
> +
> + [C] 'function void victim(type_changed*)' has some indirect sub-type changes:
> + parameter 1 of type 'type_changed*' has sub-type changes:
> + in pointed to type 'struct type_changed':
> + type size changed from 32 to 64 (in bits)
> + 1 data member change:
> + type of 'int type_changed::x' changed:
> + type name changed from 'int' to 'long int'
> + type size changed from 32 to 64 (in bits)
> +
> +1 Removed variable:
> +
> + [D] 'int var_removed' {var_removed}
> +
> +1 Added variable:
> +
> + [A] 'long int var_added' {var_added}
> +
> +1 Changed variable:
> +
> + [C] 'int var_changed' was changed to 'long int var_changed':
> + size of symbol changed from 4 to 8
> + type of variable changed:
> + type name changed from 'int' to 'long int'
> + type size changed from 32 to 64 (in bits)
> +
> diff --git a/tests/data/test-abidiff-exit/test-net-change-report1.txt b/tests/data/test-abidiff-exit/test-net-change-report1.txt
> new file mode 100644
> index 0000000..4dd6096
> --- /dev/null
> +++ b/tests/data/test-abidiff-exit/test-net-change-report1.txt
> @@ -0,0 +1,3 @@
> +Functions changes summary: 0 Removed (1 filtered out), 0 Changed (2 filtered out), 0 Added (1 filtered out) functions
> +Variables changes summary: 0 Removed (1 filtered out), 0 Changed (1 filtered out), 0 Added (1 filtered out) variables
> +
> diff --git a/tests/data/test-abidiff-exit/test-net-change-report2.txt b/tests/data/test-abidiff-exit/test-net-change-report2.txt
> new file mode 100644
> index 0000000..ca3b3e0
> --- /dev/null
> +++ b/tests/data/test-abidiff-exit/test-net-change-report2.txt
> @@ -0,0 +1,42 @@
> +Leaf changes summary: 7 artifacts changed
> +Changed leaf types summary: 1 leaf type changed
> +Removed/Changed/Added functions summary: 1 Removed, 1 Changed, 1 Added function
> +Removed/Changed/Added variables summary: 1 Removed, 1 Changed, 1 Added variable
> +
> +1 Removed function:
> +
> + [D] 'function int fun_removed()' {fun_removed}
> +
> +1 Added function:
> +
> + [A] 'function long int fun_added()' {fun_added}
> +
> +1 function with some sub-type change:
> +
> + [C] 'function int fun_changed()' has some sub-type changes:
> + return type changed:
> + type name changed from 'int' to 'long int'
> + type size changed from 32 to 64 (in bits)
> +
> +1 Removed variable:
> +
> + [D] 'int var_removed' {var_removed}
> +
> +1 Added variable:
> +
> + [A] 'long int var_added' {var_added}
> +
> +1 Changed variable:
> +
> + [C] 'int var_changed' was changed to 'long int var_changed':
> + size of symbol changed from 4 to 8
> + type of variable changed:
> + type name changed from 'int' to 'long int'
> + type size changed from 32 to 64 (in bits)
> +
> +'struct type_changed' changed:
> + type size changed from 32 to 64 (in bits)
> + there are data member changes:
> + type 'int' of 'type_changed::x' changed:
> + type name changed from 'int' to 'long int'
> + type size changed from 32 to 64 (in bits)
> diff --git a/tests/data/test-abidiff-exit/test-net-change-report3.txt b/tests/data/test-abidiff-exit/test-net-change-report3.txt
> new file mode 100644
> index 0000000..4856a77
> --- /dev/null
> +++ b/tests/data/test-abidiff-exit/test-net-change-report3.txt
> @@ -0,0 +1,5 @@
> +Leaf changes summary: 0 artifact changed (3 filtered out)
> +Changed leaf types summary: 0 (1 filtered out) leaf type changed
> +Removed/Changed/Added functions summary: 0 Removed (1 filtered out), 0 Changed (1 filtered out), 0 Added function (1 filtered out)
> +Removed/Changed/Added variables summary: 0 Removed (1 filtered out), 0 Changed (1 filtered out), 0 Added variable (1 filtered out)
> +
> diff --git a/tests/data/test-abidiff-exit/test-net-change-v0.c b/tests/data/test-abidiff-exit/test-net-change-v0.c
> new file mode 100644
> index 0000000..684b9c6
> --- /dev/null
> +++ b/tests/data/test-abidiff-exit/test-net-change-v0.c
> @@ -0,0 +1,14 @@
> +int var_removed = 0;
> +int fun_removed() { return 0; }
> +
> +int var_changed = 0;
> +int fun_changed() { return 0; }
> +
> +struct type_removed {
> + int x;
> +};
> +struct type_changed {
> + int x;
> +};
> +
> +void victim(struct type_changed * dummy) { (void)dummy->x; }
> diff --git a/tests/data/test-abidiff-exit/test-net-change-v0.o b/tests/data/test-abidiff-exit/test-net-change-v0.o
> new file mode 100644
> index 0000000000000000000000000000000000000000..bd2c744744880e83ba295ba16995c32a7438ec53
> GIT binary patch
> literal 3512
> zcmbtW&2Jk;6o2cD6Q^!mCoa$^MRE(M6uRp~R3%MGYY3zXA611^sS;G_+B>#a*t^ks
> z9h_DOwL(a}6$z<D;#MSXh$FpK>Xkcx0US6W;8ei@-kY5lPbM32;7OkM=J$T@&CHvR
> z^`~#WyPC5sV6osljCza$JSmLqx-{!hff=~6cIW3GsDJITVXocz<jLJ@kJ0rD{>F#{
> z1)2Jww6*4#X6MMZto#b9Nt+$^SUw07NS$MbQ|2DC4ivVLo*#l$oS<lcacZoRRAd&Z
> zL9xF67TXDFmYyR!_eBQ^h3jZdN;EHrAd_SimwI#ic)^;sUMN+Hc4@9OZOu%SpTW$h
> z!7eQo&lS%W*DDuhb4LK|=b>0UU$n8LV-?FT6^p6gs^Ijez%DR-5eFW3tqZf9;lwPZ
> ztslAf#w?L1@>*`nTsVZ8*U>ZS!YFFJvJ-`iIfXx2N6@wuF?|!TyCWZ<Wy8zZTGrpC
> zDQx9&!JV`q5&gsoMdCC&Uf34S*1X%4f%ytdrh{O&l0W)(-nyEvEGXk8d(zDxzMZo%
> zS!)n(#G)5%2_LqCW)k$EHt5H-Zq)R;wRRM>yQ0>KdZO0ejH71Ydo?fg<0$ZJ-Jsz$
> zg0>fQYc!F5&G!;dnhxTA&}uni7$o2idcAGf^5Ttjnm#1kn_|?-S_Dmt*%vQesje)%
> zT0QH&;?7r{UNQ*9a$AHV4w~5Y;$~-gxOm1nbGquZt4_=7Z->oJ9EH)K?+n6^gV1*p
> zuhA8KISF1ZTwGaMs=oN9XaruECU+K3rUo3AWyBiacpt1`*_t~zQMijWKZ-d0NaV46
> zy;S}@w=y-piUh>S_Zm4-_Va(3x-<RCwMXI5c!&S^XO9B`+{P>z@tq^E%WtD6OKkN@
> zm_9qb$R><g<Q2}q<u%Q~WkoXZsXZ70Hk9_k_q<^6G%}~Pb*1kez;VQ9wEFN2T*YS|
> z>m|i!9_v+w(>NrIXJ0`&>wL_1T`=tq=qFrVJj}~)<pIs6@p#3AQ*|E@h%%ijVEne~
> zKV{(fCP|r9nof*<I!-E_^62jp)ro}j|CV;)oYIs>-@SJf&ip|2uWI~5$|mJ0oB0FP
> zzhU6K{#y#?`SX6<H~8r@2=JYO-&6gEMjqbJ9}T|h!cPXDaecq?45BXY!>98_!t+mi
> z099qsp6L5hH*lWkbpz)*fZN~h;bVt7iBsKSD-pv4TzaIzZS?!##-i)FzGw{E8$sBL
> z&?jxL(TK&?F1Huc_holTHwXoX;1J+8qh3$ozXUgo65-;j?er6``H@N_IvcIn>j^dS
> z|EcCMo@&VT=X7;t9AiLf;_5<v!pxV@(@C0b#-BhR-4iq3S26k>B8cA?<%R_`%$(%o
> z_+Oc*w8=e%IOVJ>KdZ7T$oDFM&LQ!5KKLg^$}+ZQPI6pQ;5U*rYE15BbeR~j|5V`o
> zC{$V|m)?o2`9D&h6pp9)6J*ZcN6gHL5FA&RXM;MLm|XH_jsLqEpX)^9(|c=<zlj*-
> z<aweg<(3-%F9ww6@mb$QJS+aDI#AF4NBn0h-k`yK^FS&BkIVe8RJ{JykdNbsOjMfJ
> z&+wOMWYw>rh-->S^(*0z<GHRhA9HL9DN|AuKEL#Rqwk9uPkWNJ)JN(Oeh>Y+nCqv!
> QS@D<EKo9g_w9WW`0Y2;*ng9R*
>
> literal 0
> HcmV?d00001
>
> diff --git a/tests/data/test-abidiff-exit/test-net-change-v1.c b/tests/data/test-abidiff-exit/test-net-change-v1.c
> new file mode 100644
> index 0000000..7b2a4ad
> --- /dev/null
> +++ b/tests/data/test-abidiff-exit/test-net-change-v1.c
> @@ -0,0 +1,14 @@
> +long var_added = 0;
> +long fun_added() { return 0; }
> +
> +long var_changed = 0;
> +long fun_changed() { return 0; }
> +
> +struct type_added {
> + long x;
> +};
> +struct type_changed {
> + long x;
> +};
> +
> +void victim(struct type_changed * dummy) { (void)dummy->x; }
> diff --git a/tests/data/test-abidiff-exit/test-net-change-v1.o b/tests/data/test-abidiff-exit/test-net-change-v1.o
> new file mode 100644
> index 0000000000000000000000000000000000000000..994ca58ecc5c35e7ecc51da87b685379c2734d6d
> GIT binary patch
> literal 3552
> zcmbtW-D@0G6hE`mO*Y+Tn`EOV#KIImYO6DwV2NpJ+qKa&Q4uK=3mVedo!Q;N-I*{u
> z8#fgZr3%HD3W5qEh=M+ekNVsfAAIo5zd%vnL=eFTJ?GB3vo|-BClAa$=lsss{l4?&
> zGtaLV3<D$?a1lnDL;<Xckz5yI9jY(`*EVkd{4MEQcQtY2_8WiS*}5xy0e(S#M93wP
> z{utvKJDEuU1(FS8Yz^6jDGg?+NMdR{q8UZ*l5w=Ohc3k-808X01B_8)R0JcjObwFp
> z$(PuU3$gM5$%XejP%7O(NsQGfiW1~NG>S{Dg+r`hOdAhXs%5jXP?<Jn#;5MX%xA%@
> ztd`H0&y_c;m*xv|0Oo^GE?-11oR+F#R?6knZd9@RlVFyl`wA8wbBs&#GQ+8PN?V`%
> z@bWx4j~A8Pl(}>qGjF10!huoLW;Kh#!90eam`5nl*fLTVAp=o(89x&q#nv$XsZ3%k
> zwhXSM0g2yF?9fl_X2%WNzP-ETG(~K21}4%Xs8)*$&lZjA#o49e2_c*@C!FH!+XWMo
> z?FP*x=s|7Jk89ni>2_=FC~9~8S|{rHwf0UNH3QGBxuF+FfmiDW4Yv`r-Jn~e;{BTE
> zCaw@|#J!-^vi)I@fH&y%_F&hIx6`h9knHXFBPFln-6(8ZL70dz*YkW2uDo!~T3dR;
> zI_Es@ELwIi8HE0YwjcU&&_vaZo1F{8m1TSRtYx<?yXE%x!e%Fq!f4R92jOc$=-G+e
> z==yzV4Z`g-DM_+daAj?6)q3P<zY(}$TE<>EoeE+mv6*GTxOo+f;gqp(Y`k;_=l_1h
> z=_3+b^r_0!yM?vMv2}DnjBHPm5$Q1dE2%lx&ZYwlOXGR|<KF`Y1aL|7V8pZSu+MIy
> zC67$LF?5@m9%>!NEaEul;L7EtSk3HED<Fy{OzsC*GXYO)4w-P^HiLW7Z(13bJZBc-
> zGs^skH9_nQ`%6E`m)T$XNq(7eszW64{NqSxV6^X2Nj(?pZsfz5rp;#hDC*fAin1Ir
> z;WWFS;zu-{GLZN#>-T85T&a1+X~(F$<Fw*0=Yk+d!Kv;Cte<C2dDPWgXI%E@J=UMo
> z@GaJ_Gp9VqStjPPEb|1c-_&qc;Vl*@4~<K%(?^WUJTI~SwTA!6`ghD_9=XmxX#T&k
> z{*&g<YPNP%S3IL^m^#FN^$^@*oW`WCS6#zpKW}KboCk3Fdp*1rktcD=J5oygVFC_a
> z>fki`eQ;vG>pGs_7__&8uoa<A$Zn$%`@8$hZcJ~`{*Z1E`Z5GdfYXe6Js+=lCyWx`
> z!Ry`bCvNjqPULsCTd~{ox#RznEnqyYoqBdCWt|$NlPSwu@>PlKgy}b=t2!yN9)AjL
> zbbj>s4#&v#mnqb<M%WU5dQQS+{O_bFr2EpJ2K}4W6(15Uo{;Z3fcimxvOfOE2)cl+
> zo|6n$8T>+cjT+teMKtLcdH==W;wV%|x-Z=mdHw&DFEANT{U=E8zlWHf6Crq*56?C!
> z3hBON&#OPjh2%U@eYy|z`a6hGPT5Zs1---d^#@$mm;80~&Wpdr8|nn-lT5}*ev9L!
> zUGmTL4ao)>C;dJ~fiUi_gyAcWA7)4@$ni^<-iN&TTV%%#cBJ{E*ImZTdCjX$Ar>V?
> o`i479?;gE7dOWR3-cWz!OL!CQD(T~=ym|3MF7&w?jItj8AJ|S9SpWb4
>
> literal 0
> HcmV?d00001
>
> diff --git a/tests/data/test-abidiff-exit/test-net-change.abignore b/tests/data/test-abidiff-exit/test-net-change.abignore
> new file mode 100644
> index 0000000..6ba2118
> --- /dev/null
> +++ b/tests/data/test-abidiff-exit/test-net-change.abignore
> @@ -0,0 +1,8 @@
> +[suppress_function]
> + name_regexp = ^fun_
> +
> +[suppress_variable]
> + name_regexp = ^var_
> +
> +[suppress_type]
> + name_regexp = ^type_
> diff --git a/tests/test-abidiff-exit.cc b/tests/test-abidiff-exit.cc
> index 244de2a..936d1d6 100644
> --- a/tests/test-abidiff-exit.cc
> +++ b/tests/test-abidiff-exit.cc
> @@ -239,6 +239,44 @@ InOutSpec in_out_specs[] =
> "data/test-abidiff-exit/test-decl-enum-report-3.txt",
> "output/test-abidiff-exit/test-decl-enum-report-3.txt"
> },
> + {
> + "data/test-abidiff-exit/test-net-change-v0.o",
> + "data/test-abidiff-exit/test-net-change-v1.o",
> + "",
> + "--no-default-suppression --no-show-locs",
> + abigail::tools_utils::ABIDIFF_ABI_CHANGE
> + | abigail::tools_utils::ABIDIFF_ABI_INCOMPATIBLE_CHANGE,
> + "data/test-abidiff-exit/test-net-change-report0.txt",
> + "output/test-abidiff-exit/test-net-change-report0.txt"
> + },
> + {
> + "data/test-abidiff-exit/test-net-change-v0.o",
> + "data/test-abidiff-exit/test-net-change-v1.o",
> + "data/test-abidiff-exit/test-net-change.abignore",
> + "--no-default-suppression --no-show-locs",
> + abigail::tools_utils::ABIDIFF_OK,
> + "data/test-abidiff-exit/test-net-change-report1.txt",
> + "output/test-abidiff-exit/test-net-change-report1.txt"
> + },
> + {
> + "data/test-abidiff-exit/test-net-change-v0.o",
> + "data/test-abidiff-exit/test-net-change-v1.o",
> + "",
> + "--no-default-suppression --no-show-locs --leaf-changes-only",
> + abigail::tools_utils::ABIDIFF_ABI_CHANGE
> + | abigail::tools_utils::ABIDIFF_ABI_INCOMPATIBLE_CHANGE,
> + "data/test-abidiff-exit/test-net-change-report2.txt",
> + "output/test-abidiff-exit/test-net-change-report2.txt"
> + },
> + {
> + "data/test-abidiff-exit/test-net-change-v0.o",
> + "data/test-abidiff-exit/test-net-change-v1.o",
> + "data/test-abidiff-exit/test-net-change.abignore",
> + "--no-default-suppression --no-show-locs --leaf-changes-only",
> + abigail::tools_utils::ABIDIFF_OK,
> + "data/test-abidiff-exit/test-net-change-report3.txt",
> + "output/test-abidiff-exit/test-net-change-report3.txt"
> + },
> {0, 0, 0 ,0, abigail::tools_utils::ABIDIFF_OK, 0, 0}
> };
>
> --
> 1.8.3.1
>
>
> --
> Dodji
Thank you for the review and follow-up!
Regards,
Giuliano.
next prev parent reply other threads:[~2020-07-17 16:35 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-24 17:13 [RFC] Fix has_net_changes for leaf-changes-only mode Giuliano Procida
2020-03-24 17:17 ` Giuliano Procida
2020-03-26 10:37 ` Matthias Maennich
2020-03-26 12:01 ` Giuliano Procida
2020-03-26 17:12 ` Dodji Seketeli
2020-03-27 18:42 ` Giuliano Procida
2020-03-27 19:01 ` [PATCH v2] Fix has_net_changes for --leaf-changes-only mode Giuliano Procida
2020-03-30 9:17 ` Matthias Maennich
2020-07-01 13:19 ` [PATCH v3 0/1] Fix abidiff exit code when diffs are suppressed Giuliano Procida
2020-07-01 13:19 ` [PATCH v3 1/1] Fix has_net_changes for --leaf-changes-only mode Giuliano Procida
2020-07-17 13:54 ` Dodji Seketeli
2020-07-17 16:34 ` Giuliano Procida [this message]
2020-07-21 6:28 ` Dodji Seketeli
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAGvU0H=gSM_2qnf9TtNcLrRvp0_N_oMDcexFVLPRuRtSrsQMgQ@mail.gmail.com' \
--to=gprocida@google.com \
--cc=dodji@seketeli.org \
--cc=kernel-team@android.com \
--cc=libabigail@sourceware.org \
--cc=maennich@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).