On Wed, 2 Jun 2021 13:24:00 GMT, Erik Joelsson <er...@openjdk.org> wrote:
>> Nikita Gubarkov has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 8267706: Break long line in make/ide/idea/jdk/idea.gmk > > We do not require brew to install packages on mac, the build doc references > brew as an easy way to get autoconf. I personally have always built autoconf > from source instead (as it's a very simple build that takes a few seconds at > most). I agree that adding a new dependency on the build is troublesome. We > generally try really hard to avoid depending on tools that aren't default > available on all the main build environments. I didn't check this review very > closely for such things. I also did say that I wanted people who actually use > this script to weigh in, which I didn't see happen before it was integrated. > > Looking at the patch again, the first realpath use can easily be worked > around by generating the relative path to SPEC_DIR from idea.gmk. In make we > have a macro to generate relative paths, see RelativePath in > common/Utils.gmk. The second realpath is operating on moduleSrcDirs, also > generated in idea.gmk. The same thing can be applied here, the RelativePath > macro together with a foreach can be used to export the src dirs as relative > paths. > > In general, I think we would have been much better off if this script was > just written in make from the start, as we have more platform independent > tools available there already. Rewriting it isn't trivial though, and > requires quite a bit of knowledge both of GNU make as well as our macros, so > I'm not asking for this to happen. I do think we should eliminate the use of > realpath though. @erikj79 thank you for the hint about `RelativePath` macro, I will rewrite this logic and get rid of `realpath`. Can I reference the same issue in new PR, or do I need a new one? ------------- PR: https://git.openjdk.java.net/jdk/pull/4190
