| Message ID | yjt21ufi7h5d.fsf@ruffy2.mtv.corp.google.com |
|---|---|
| State | New |
| Headers | show |
On Sat, Nov 24, 2012 at 5:47 PM, Doug Evans <dje@google.com> wrote: > > Hi. > This third patch passes options.results to GetSumFiles when fetching the > results > for the clean build. > It is useful in my use cases, but I'm not sure it's useful for upstream. > [An alternative is to add another option to specify the results file(s) > for the clean > build, but I'm being conservative and not adding an option if I don't have > to.] > > Ok to check in? > > 2012-11-24 Doug Evans <dje@google.com> > > * testsuite-management/validate_failures.py (CompareBuilds): Pass > options.results > to GetSumFiles for clean build. I guess I don't understand your use case. CompareBuilds is for when you have a clean build that you want to crawl for failures to use as your comparison base. If you have a manifest already, why would you be using --clean_build? Diego.
On Sun, Nov 25, 2012 at 7:40 AM, Diego Novillo <dnovillo@google.com> wrote: > On Sat, Nov 24, 2012 at 5:47 PM, Doug Evans <dje@google.com> wrote: >> >> Hi. >> This third patch passes options.results to GetSumFiles when fetching the >> results >> for the clean build. >> It is useful in my use cases, but I'm not sure it's useful for upstream. >> [An alternative is to add another option to specify the results file(s) >> for the clean >> build, but I'm being conservative and not adding an option if I don't have >> to.] >> >> Ok to check in? >> >> 2012-11-24 Doug Evans <dje@google.com> >> >> * testsuite-management/validate_failures.py (CompareBuilds): Pass >> options.results >> to GetSumFiles for clean build. > > I guess I don't understand your use case. CompareBuilds is for when > you have a clean build that you want to crawl for failures to use as > your comparison base. If you have a manifest already, why would you > be using --clean_build? In gdb-land, parallel make check runs collect all the subdirectory .sum files and reconstruct testsuite/${tool}.sum. There's more than one solution of course: alternatively one could have gdb stop doing this. But I'm not sure which is better, and rather than change gdb I went for changing validate_failure.py (which made sense regardless of what gdb is doing: use the same .sum files in the comparison). Could be missing something of course. :-)
On Wed, Nov 28, 2012 at 1:55 PM, Doug Evans <dje@google.com> wrote: > In gdb-land, parallel make check runs collect all the subdirectory > .sum files and reconstruct testsuite/${tool}.sum. > > There's more than one solution of course: alternatively one could have > gdb stop doing this. But I'm not sure which is better, and rather > than change gdb I went for changing validate_failure.py (which made > sense regardless of what gdb is doing: use the same .sum files in the > comparison). Could be missing something of course. :-) So, you do have a set of .sum files, right? You could feed them to validate_failures.py via --manifest=collected_results_from_gdb.sum. You do not need to use the --clean_build flag. What I'm saying is that from a user perspective, it doesn't really make sense to use --clean_build and --manifest together. If you have the --manifest, you do not need to go to --clean_build to look for the .sum files. Am I right in understanding that you are intending to call $ validate_failures.py --clean_build=/path/to/gdb/bld --manifest=/path/to/gdb.sum ? If that's not what you are intending, then I am completely lost :) Diego.
On Wed, Nov 28, 2012 at 12:35 PM, Diego Novillo <dnovillo@google.com> wrote: > On Wed, Nov 28, 2012 at 1:55 PM, Doug Evans <dje@google.com> wrote: > >> In gdb-land, parallel make check runs collect all the subdirectory >> .sum files and reconstruct testsuite/${tool}.sum. >> >> There's more than one solution of course: alternatively one could have >> gdb stop doing this. But I'm not sure which is better, and rather >> than change gdb I went for changing validate_failure.py (which made >> sense regardless of what gdb is doing: use the same .sum files in the >> comparison). Could be missing something of course. :-) > > So, you do have a set of .sum files, right? You could feed them to > validate_failures.py via --manifest=collected_results_from_gdb.sum. > You do not need to use the --clean_build flag. > > What I'm saying is that from a user perspective, it doesn't really > make sense to use --clean_build and --manifest together. If you have > the --manifest, you do not need to go to --clean_build to look for the > .sum files. > > Am I right in understanding that you are intending to call > > $ validate_failures.py --clean_build=/path/to/gdb/bld > --manifest=/path/to/gdb.sum ? > > If that's not what you are intending, then I am completely lost :) s/--manifest/--results/ i.e. $ validate_failures.py --clean_build=/path/to/gdb/bld --results=/path/to/gdb.sum ...
On Wed, Nov 28, 2012 at 6:02 PM, Doug Evans <dje@google.com> wrote: > On Wed, Nov 28, 2012 at 12:35 PM, Diego Novillo <dnovillo@google.com> wrote: >> >> Am I right in understanding that you are intending to call >> >> $ validate_failures.py --clean_build=/path/to/gdb/bld >> --manifest=/path/to/gdb.sum ? >> >> If that's not what you are intending, then I am completely lost :) > > s/--manifest/--results/ > > i.e. > $ validate_failures.py --clean_build=/path/to/gdb/bld > --results=/path/to/gdb.sum ... Aha! OK, yes, that makes sense. I'm sorry it took me so long to get this. At first I was almost convinced that the patch was going to break my favourite use: $ validate_failures.py --clean_build=../clean/bld The patch is fine. Thanks. Diego.
--- validate_failures.py= 2012-11-19 11:47:29.997632548 -0800 +++ validate_failures.py 2012-11-24 13:26:07.727726123 -0800 @@ -373,7 +373,7 @@ def CompareBuilds(options): sum_files = GetSumFiles(options.results, options.build_dir) actual = GetResults(sum_files) - clean_sum_files = GetSumFiles(None, options.clean_build) + clean_sum_files = GetSumFiles(options.results, options.clean_build) clean = GetResults(clean_sum_files) return PerformComparison(clean, actual, options.ignore_missing_failures)