On Dec 19 10:04, Thirumalai Nagalingam wrote:
> 
> > Just one nit for now.  The first patch contains newlib/Makefile.in but not 
> > newlib/Makefile.am.  
> > Please make sure to change Makefile.am instead.
> 
> Hi Corinna,
>  
> Thank you for the review.
>  
> The changes in Makefile.in is directly due to the Cygwin build workflow[1] 
> (cygwin.yml), which does not run a configure step for
> the newlib subtree, unlike what is done for the winsup directory. 
> As a result, the changes made in Makefile.inc (as in this case) would not 
> propagate into the build.
>  
> The functional changes themselves are confined to newlib/libm/Makefile.inc, 
> which is included by newlib/Makefile.am,
> and therefore, would normally be reflected in the generated 
> newlib/Makefile.in.
> However, since the newlib subtree is not reconfigured in workflow, the 
> corresponding updates
> were applied directly to Makefile.in which were regenerated using automake in 
> local setup.
>  
> [1]- 
> https://github.com/cygwin/cygwin/blob/9fac993ba74bd6ab3fc638a169ffdc92b78bd679/.github/workflows/cygwin.yml#L150

Ok, got it.  But, as for the workflow:

You should create patches for newlib separately from patches for Cygwin
and send them to the newlib mailing list.  Given the order in which
things are built in the repo, it's usually better to send the newlib
stuff prior to sending any changes to Cygwin which are affected by the
newlib change.

The patch changing the affected Makefile.inc files should contain only
the changes to the inc file.  Jeff or I will then regenerate the
Makefile.in and configure files as necessary.

The advantage is that you can seperate the newlib stuff out and nobody
has to review generated files, only the input files.


Thanks,
Corinna

Reply via email to