| Message ID | EFA604B5-13BA-4265-AAE6-DCB791C250EE@adacore.com |
|---|---|
| State | New |
| Headers | show |
Hi Tristan, On Mon, May 16, 2011 at 6:20 AM, Tristan Gingold <gingold@adacore.com> wrote: > Hi, > > this rule (and only this one) uses the c function of sed. According to man, the syntax should be: > > [2addr]c\ > text > > (Note the trailing back-slash). > But as currently written, there is no backslash in it. As a consequence 'make check' fails at least on Darwin (BSD derived sed > command). I think it would be better to stick with the common syntax, as done by this patch. > > Tested with 'make check'. Tested with 'make check' on which platforms? I am sure it includes Darwin (iOS/OSX/Apple), but assurance is needed about other flavors of "sed" e.g., GNU Solaris HP/UX AIX > Ok for trunk ? Probably. :) Just assure us it is properly tested.
On May 16, 2011, at 3:40 PM, Bruce Korb wrote: > Hi Tristan, > > On Mon, May 16, 2011 at 6:20 AM, Tristan Gingold <gingold@adacore.com> wrote: >> Hi, >> >> this rule (and only this one) uses the c function of sed. According to man, the syntax should be: >> >> [2addr]c\ >> text >> >> (Note the trailing back-slash). >> But as currently written, there is no backslash in it. As a consequence 'make check' fails at least on Darwin (BSD derived sed >> command). I think it would be better to stick with the common syntax, as done by this patch. >> >> Tested with 'make check'. > > Tested with 'make check' on which platforms? > I am sure it includes Darwin (iOS/OSX/Apple), but assurance is needed about > other flavors of "sed" e.g., > GNU > Solaris > HP/UX > AIX Yes, it is ok on GNU/Linux and Solaris. Do I really need to test on HP/UX and AIX ? Won't be easy for me. Tristan. >> Ok for trunk ? > > Probably. :) Just assure us it is properly tested.
Hi Tristan, On Mon, May 16, 2011 at 8:27 AM, Tristan Gingold <gingold@adacore.com> wrote: > Yes, it is ok on GNU/Linux and Solaris. > > Do I really need to test on HP/UX and AIX ? Won't be easy for me. Solaris and BSD are usually the most unusual, so I'd say go ahead for mainline and see if you get squawks. With several weeks of silence, I'd then be inclined to back port to the active branch(es) since it is actually a bug fix. (Which also leaves me wondering how it has worked up until now.....)
On May 16, 2011, at 5:45 PM, Bruce Korb wrote: > Hi Tristan, > > On Mon, May 16, 2011 at 8:27 AM, Tristan Gingold <gingold@adacore.com> wrote: >> Yes, it is ok on GNU/Linux and Solaris. >> >> Do I really need to test on HP/UX and AIX ? Won't be easy for me. > > Solaris and BSD are usually the most unusual, so I'd say go ahead for mainline > and see if you get squawks. With several weeks of silence, I'd then be inclined > to back port to the active branch(es) since it is actually a bug fix. Ok, thanks. > (Which also > leaves me wondering how it has worked up until now.....) Well GNU sed is tolerant. And may be installed on some solaris machines. Tristan.
Hello, * Tristan Gingold wrote on Mon, May 16, 2011 at 03:20:19PM CEST: > Ok for trunk ? FWIW this looks good to me, and certainly necessary for Posix and portable sed, but I can't approve it. Thanks, Ralf > fixincludes/ > 2011-05-16 Tristan Gingold <gingold@adacore.com> > > * inclhack.def (solaris_complex_cxx): Fix syntax. > * fixincl.x: Regenerate. > > diff --git a/fixincludes/inclhack.def b/fixincludes/inclhack.def > index ac8f795..a20ab9d 100644 > --- a/fixincludes/inclhack.def > +++ b/fixincludes/inclhack.def > @@ -3315,9 +3315,9 @@ fix = { > hackname = solaris_complex_cxx; > mach = "*-*-solaris2.*"; > files = complex.h; > - sed = "/#if[ \t]*!defined(__cplusplus)/c" > + sed = "/#if[ \t]*!defined(__cplusplus)/c\\\n" > "#ifdef\t__cplusplus\\\nextern \"C\" {\\\n#endif"; > - sed = "/#endif[ \t]*\\/\\* !defined(__cplusplus) \\*\\//c" > + sed = "/#endif[ \t]*\\/\\* !defined(__cplusplus) \\*\\//c\\\n" > "#ifdef\t__cplusplus\\\n}\\\n#endif"; > test_text = "#if !defined(__cplusplus)\n" > "#endif /* !defined(__cplusplus) */"; >
diff --git a/fixincludes/inclhack.def b/fixincludes/inclhack.def index ac8f795..a20ab9d 100644 --- a/fixincludes/inclhack.def +++ b/fixincludes/inclhack.def @@ -3315,9 +3315,9 @@ fix = { hackname = solaris_complex_cxx; mach = "*-*-solaris2.*"; files = complex.h; - sed = "/#if[ \t]*!defined(__cplusplus)/c" + sed = "/#if[ \t]*!defined(__cplusplus)/c\\\n" "#ifdef\t__cplusplus\\\nextern \"C\" {\\\n#endif"; - sed = "/#endif[ \t]*\\/\\* !defined(__cplusplus) \\*\\//c" + sed = "/#endif[ \t]*\\/\\* !defined(__cplusplus) \\*\\//c\\\n" "#ifdef\t__cplusplus\\\n}\\\n#endif"; test_text = "#if !defined(__cplusplus)\n" "#endif /* !defined(__cplusplus) */";
Hi, this rule (and only this one) uses the c function of sed. According to man, the syntax should be: [2addr]c\ text (Note the trailing back-slash). But as currently written, there is no backslash in it. As a consequence 'make check' fails at least on Darwin (BSD derived sed command). I think it would be better to stick with the common syntax, as done by this patch. Tested with 'make check'. Ok for trunk ? Tristan. fixincludes/ 2011-05-16 Tristan Gingold <gingold@adacore.com> * inclhack.def (solaris_complex_cxx): Fix syntax. * fixincl.x: Regenerate.