public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jacob Bachmeyer <jcb62281@gmail.com>
To: Maciej Rozycki <macro@wdc.com>
Cc: "gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>,
	 "dejagnu@gnu.org" <dejagnu@gnu.org>,
	Pierre-Marie de Rodat <derodat@adacore.com>,
	 Arnaud Charlet <charlet@adacore.com>,
	Eric Botcazou <ebotcazou@libertysurf.fr>
Subject: Re: [PATCH 3/3][DejaGNU] target: Wrap linker flags into `-largs'/`-margs' for Ada
Date: Wed, 22 May 2019 00:04:00 -0000	[thread overview]
Message-ID: <5CE49202.6010705@gmail.com> (raw)
In-Reply-To: <alpine.DEB.2.20.1905171531270.18422@tpp.hgst.com>

Maciej Rozycki wrote:
> On Thu, 16 May 2019, Jacob Bachmeyer wrote:
>
>   
>>>  I suspect the origins may be different, however as valuable as your 
>>> observation is functional problems have precedence over issues with code 
>>> structuring, so we need to fix the problem at hand first.  I'm sure 
>>> DejaGNU maintainers will be happy to review your implementation of code 
>>> restructuring afterwards.
>>>       
>> My concern is that your patch may only solve a small part of the problem 
>> -- enough to make your specific case work, yes, but then someone else 
>> will hit other parts of the problem later and we spiral towards "tissue 
>> of hacks" unmaintainability.
>>     
>
>  I think however that fixing problems in small steps as they are 
> discovered is also a reasonable approach and a way to move forward: 
> perfect is the enemy of good.
>   

Fair enough; observe the small patches I have recently submitted to DejaGnu.

>  So I don't think the prospect of making a comprehensive solution should 
> prevent a simple fix for the problem at hand that has been already 
> developed from being applied.
>   

I recognize a difference between "simple but complete" (an ideal 
sometimes achieved in practice) and "simple because incomplete" (which 
does not actually fix the problem).  My concerns are that your patch may 
be the latter.

>  IOW I don't discourage you from developing a comprehensive solution, 
> however applying my proposal right away will help at least some people and 
> will not block you in any way.
>   

Correct, although, considering how long my FSF paperwork took, I might 
be able to finish a comprehensive patch before your paperwork is 
completed.  :-)

>> The biggest hint to me that your patch is incomplete is that it only 
>> adds -largs/-margs to wrap LDFLAGS.  I suspect that there are other 
>> -?args options that should be used also with other flag sets, but those 
>> do not appear in this patch.  Do we know what the GNU Ada frontend 
>> actually expects?
>>     
>
>  At first glance it looks to me we should be safe overall as compiler 
> flags are supposed to be passed through by `gnatmake' (barring switch 
> processing bugs, as observed with 1/3), and IIUC assembler flags are 
> considered compiler flags for the purpose of this consideration as 
> `gnatmake' does not make assembly a separate build stage.  So while we 
> could wrap compiler flags into `-cargs'/`-margs', it would only serve to 
> avoid possible `gnatmake' switch processing bugs.
>   

I am not sure if those are actually bugs in `gnatmake' or the result of 
an incomplete specification for `gnatmake' -- I suspect that --sysroot= 
may have been added to the rest of GCC after `gnatmake' was written and 
whoever added it did not update the Ada frontend.

>  There's also `-bargs' for binder switches, but I can't see any use for it 
> here.
>
>  Finally boards are offered the choice of `adaflags', `cflags', 
> `cxxflags', etc. for the individual languages, where the correct syntax 
> can be used if anything unusual is needed beyond what I have noted above.
>   

Which also raises the issue of "cflags_for_target" (used regardless of 
language and currently always taken from the "unix" board configuration) 
and how that is supposed to make sense and whether it should be 
similarly split into language-specific values or simply removed.  I have 
already submitted a patch to draw that value from the actual host board 
configuration.

>  I'll defer any further consideration to the Ada maintainers cc-ed; I do 
> hope I haven't missed anything here, but then Ada is far from being my 
> primary area of experience.
>   

Likewise, hopefully some of the Ada maintainers will be able to shed 
light on this issue.  And I hope Ben (the DejaGnu maintainer) is okay -- 
I would have expected him to comment by now.

>>>  The ordering rules are system-specific I'm afraid and we have to be 
>>> careful not to break working systems out there.  People may be forced to a 
>>> DejaGNU upgrate, due to a newer version of a program being tested having 
>>> such a requirement, and can legitimately expect their system to continue 
>>> working.
>>>       
>> We can start by simply preserving the existing ordering until we know 
>> something should change, but the main goal of my previous message was to 
>> collect the requirements for a specification for default_target_compile 
>> so I can write regression tests (some of which will fail due to known 
>> bugs like broken Ada support in our current implementation) before 
>> embarking on extensive changes to that procedure.  Improving 
>> "target.test" was already on my local TODO list.
>>     
>
>  You are welcome to go ahead with your effort as far as I am concerned.
>   

I am working on it.  :-)

-- Jacob

  reply	other threads:[~2019-05-22  0:04 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-14 21:46 [PATCH 0/3] GNAT test suite fixes for build sysroot Maciej W. Rozycki
2019-05-14 21:47 ` [PATCH 1/3][GCC] gnatmake: Accept the `--sysroot=' GCC driver option Maciej W. Rozycki
2019-05-14 21:48 ` [PATCH 2/3][GCC] GNAT/testsuite: Pass the `ada' option to target compilation Maciej W. Rozycki
2019-05-15 23:12   ` Jacob Bachmeyer
2019-05-16 12:38     ` Maciej W. Rozycki
2019-05-16 22:57       ` Jacob Bachmeyer
2019-05-14 21:49 ` [PATCH 3/3][DejaGNU] target: Wrap linker flags into `-largs'/`-margs' for Ada Maciej W. Rozycki
2019-05-16  0:00   ` Jacob Bachmeyer
2019-05-16 12:58     ` Maciej W. Rozycki
2019-05-16 23:39       ` Jacob Bachmeyer
2019-05-21 21:37         ` Maciej Rozycki
2019-05-22  0:04           ` Jacob Bachmeyer [this message]
2019-10-25 17:40             ` [PING^3][PATCH " Maciej W. Rozycki
2019-10-26  1:30               ` Jacob Bachmeyer
2019-06-19 12:16 ` [PING][PATCH 0/3] GNAT test suite fixes for build sysroot Maciej Rozycki
2019-06-19 12:49   ` Arnaud Charlet
2019-06-20 14:51     ` Maciej Rozycki
2019-06-20 15:14       ` Arnaud Charlet
2019-06-20 15:32         ` Maciej Rozycki
2019-09-13 17:56 ` [PING^2][PATCH " Maciej W. Rozycki
2019-09-16  9:12   ` Arnaud Charlet
2019-09-23 23:21     ` Maciej W. Rozycki

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=5CE49202.6010705@gmail.com \
    --to=jcb62281@gmail.com \
    --cc=charlet@adacore.com \
    --cc=dejagnu@gnu.org \
    --cc=derodat@adacore.com \
    --cc=ebotcazou@libertysurf.fr \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=macro@wdc.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).