Re: [Rpm-maint] [rpm-software-management/rpm] DWARF-5 support in debugedit (#1537)
> The reason I didn't use the testcases you wrote was simply because they > depended on llvm tooling being present and you said you didn't care about > making them work with gcc. gcc did not support the needed functionality the time when I was implementing it. I sure do not care at all about gcc (like the rest of the world) but that was not the reason why I did not include the testcases for gcc. Running the tests with both gcc and clang would be a trivial extension of my testcases instead of dropping them all and replacing them by your incomplete ones. -- You are receiving this because you commented. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1537#issuecomment-779862438___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] DWARF-5 support in debugedit (#1537)
Hi Jan, On Tue, 2021-02-16 at 01:37 -0800, Jan Kratochvil wrote: > Could you rather point what was incomplete on the previous testcases > than to screw it up? You removed the only part that matters. I didn't screw up. I simply didn't use that part. But wrote some new testcases based on the existing tests, just with explicit -gdwarf-4 and -gdwarf-5 settings instead of tweaking the compiler used or needing new tools in the tests. The reason I didn't use the testcases you wrote was simply because they depended on llvm tooling being present and you said you didn't care about making them work with gcc. I just happened to be interested in testing with gcc, so I had to write some new testcases to make sure the new functionality worked as expected. ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] DWARF-5 support in debugedit (#1537)
Could you rather point what was incomplete on the previous testcases than to screw it up? You removed the only part that matters. -- You are receiving this because you commented. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1537#issuecomment-779709950___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] DWARF-5 support in debugedit (#1537)
The patchset regressed against my version as it no longer tests rpmbuild compatibility with LLVM product as required by paying customers of Red Hat company. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1537#issuecomment-779665148___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] DWARF-5 support in debugedit (#1537)
So... some of this has been reviewed in #1332 months ago and the rest has been battle-tested in rawhide for several weeks now, I don't think this PR will get any better by "letting it breath" here some more :sweat_smile: -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1537#issuecomment-779641032___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] DWARF-5 support in debugedit (#1537)
Thanks for the patches @jankratochvil and Mark! -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1537#issuecomment-779641280___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] DWARF-5 support in debugedit (#1537)
AIUI this has been real-world tested in rawhide for a few weeks now... -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1537#issuecomment-779635288___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] DWARF-5 support in debugedit (#1537)
Hi Jan,
On Tue, 2021-02-16 at 00:16 -0800, Jan Kratochvil wrote:
> The patchset regressed against my version as it no longer tests
> rpmbuild compatibility with LLVM product as required by paying
> customers of Red Hat company.
Unfortunately they did because I rewrote the tests so they didn't
require the llvm-toolset tools to run and made them independent of the
version of DWARF emitted by the compiler so that they work using either
older or newer gcc and binutils versions installed on the system.
Untested, but you can probably improve on that experience by changing
the gcc invocations inside the RPM_DEBUGEDIT_SETUP m4 macro (which is
just a shell script snippet) to use ${CC} instead. That should pick up
the CC setup by configure. That might also help people trying to build
and test rpm in a cross-compiler situation.
Cheers,
Mark
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] DWARF-5 support in debugedit (#1537)
How do I test this "in the real world"? The code looks fine (insomuch as debugedit code usually does...)... -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1537#issuecomment-779581545___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
[Rpm-maint] [rpm-software-management/rpm] DWARF-5 support in debugedit (#1537)
Resubmitting patches posted on rpm-maint: http://lists.rpm.org/pipermail/rpm-maint/2021-February/016462.html You can view, comment on, or merge this pull request online at: https://github.com/rpm-software-management/rpm/pull/1537 -- Commit Summary -- * Clean up file unpack iteration logic a bit * Drop unused filename variable * Dont update path info if rename failed on file commit * Refactor file install and remove around a common struct * Refactor fsmMkfile() to take advantage of the new state struct * Clarify file installation temporary suffix rule * Clarify file installation temporary suffix rule * Handle hardlink tracking with a file state pointer * Handle file install failures more gracefully * fixup! Handle file install failures more gracefully * fixup! Handle file install failures more gracefully * fixup! Handle file install failures more gracefully * debugedit: Protect macro arguments by parentheses * debugedit: Move code to separate functions. * debugedit: Implement DWARF-5 unit header and new forms parsing. * debugedit: Handle DWARF-5 debug_line and debug_line_str. * debugedit: Add DWARF5 tests -- File Changes -- M lib/fsm.c (341) M lib/rpmfiles.h (3) M tests/data/SPECS/hlinktest.spec (4) M tests/debugedit.at (195) M tests/rpmbuild.at (32) M tests/rpmi.at (92) M tools/debugedit.c (884) -- Patch Links -- https://github.com/rpm-software-management/rpm/pull/1537.patch https://github.com/rpm-software-management/rpm/pull/1537.diff -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1537 ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
