Re: [PATCH v2] x86: Remove libopcodes dependency

[Jan Beulich <jbeulich@suse.com>]

On 30.11.2022 01:06, H.J. Lu wrote:
> On Tue, Nov 29, 2022 at 11:38 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>>
>> On Tue, Nov 29, 2022 at 1:22 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>
>>> On 29.11.2022 00:49, H.J. Lu wrote:
>>>> On Thu, Nov 24, 2022 at 2:19 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>> On 22.11.2022 19:19, H.J. Lu wrote:
>>>>>> --- a/gas/Makefile.am
>>>>>> +++ b/gas/Makefile.am
>>>>>> @@ -446,6 +446,12 @@ development.exp: $(BFDDIR)/development.sh
>>>>>>       $(EGREP) "(development|experimental)=" $(BFDDIR)/development.sh  \
>>>>>>         | $(AWK) -F= '{ print "set " $$1 " " $$2 }' > $@
>>>>>>
>>>>>> +$(srcdir)/../opcodes/i386-init.h $(srcdir)/../opcodes/i386-tbl.h: \
>>>>>> +     @MAINT@ $(srcdir)/../opcodes/i386-opc.tbl \
>>>>>> +     $(srcdir)/../opcodes/i386-reg.tbl \
>>>>>> +     $(srcdir)/../opcodes/i386-opc.h
>>>>>> +     cd ../opcodes; make gen-i386-tbl
>>>>>
>>>>> I've made a patch to gas/Makefile.am as you have requested in reply to
>>>>> my series. I will want to put that through some more testing, so I will
>>>>> submit a v3 of that only a little later (and of course only unless you
>>>>> submit a v2 of your patch earlier that I would also end up being okay
>>>>> with). In the course of doing so I noticed a few more issues with your
>>>>> change:
>>>>>
>>>>> For one I don't think you can put @MAINT@ on a continued line, as the
>>>>> line continuation might then be hidden when @MAINT@ expands to #. The
>>>>> list of dependencies wants expressing via a variable, which would then
>>>>> be used immediately after @MAINT@ without any line continuation
>>>>> following.
>>>>
>>>> Fixed.
>>>
>>> No, the same problem is still there. You either need to use a very long
>>> line, or you need to introduce a variable holding the list of prereqs,
>>> like I've done in my series.
>>
>> I got
>>
>> $(srcdir)/../opcodes/i386-init.h $(srcdir)/../opcodes/i386-tbl.h:
>> $(srcdir)/../opcodes/i386-opc.tbl \
>>         $(srcdir)/../opcodes/i386-reg.tbl \
>>         $(srcdir)/../opcodes/i386-opc.h \
>>         $(srcdir)/../opcodes/i386-gen.c
>>         $(MAKE) -C ../opcodes gen-i386-tbl
>>
>> and
>>
>> $(srcdir)/../opcodes/i386-init.h $(srcdir)/../opcodes/i386-tbl.h: #
>> $(srcdir)/../opcodes/i386-opc.tbl \
>>         $(srcdir)/../opcodes/i386-reg.tbl \
>>         $(srcdir)/../opcodes/i386-opc.h \
>>         $(srcdir)/../opcodes/i386-gen.c
>>         $(MAKE) -C ../opcodes gen-i386-tbl
>>
>> I didn't run into any problems.
>>
>>>>> And then your rule / dependency won't be enough on a "maintainer-clean"
>>>>> tree, i.e. when the generated headers aren't there at all, and when
>>>>> config/.deps/tc-i386.Po is still empty. In that case nothing would
>>>>> trigger their generation; an explicit dependency of config/tc-i386.o on
>>>>> these headers needs adding here.
>>>>
>>>> Fixed.
>>>>
>>>>> Finally you're missing a dependency of the generated headers on
>>>>> i386-gen.c.
>>>>
>>>> They have a dependency on i386-gen which depends on i386-gen.c.
>>>
>>> In opcodes/, yes, but talk was about the rule in gas/. Yet despite
>>> your comment above I see that you've added the missing dependency.
>>>
>>>> Here is the v2 patch.
>>>
>>> As said in reply to v1 - my objection to this particular use of make
>>> recursion remains.
>>
>> The gen-i386-tbl rule doesn't touch any files used by libopcodes.
> 
> There are
> 
> bfd/Makefile.in:   ($(am__cd) $$subdir && $(MAKE) $(AM_MAKEFLAGS)
> $$local_target) \
> binutils/Makefile.in:   ($(am__cd) $$subdir && $(MAKE) $(AM_MAKEFLAGS)
> $$local_target) \
> gas/Makefile.in:   ($(am__cd) $$subdir && $(MAKE) $(AM_MAKEFLAGS)
> $$local_target) \

I'm glad you spotted this yourself - I was just about to point out
that my concern isn't only a theoretical one, because of the
generated makefiles we use. You don't draw any conclusion though:
Are you willing to accept my approach, specifically reworked to
account for an extra request of yours? Or are you meaning to make
another attempt yourself, avoiding the undue make recursion?

Jan