svarnau commented on issue #1771: [TRAFODION-3242]Do not make again before MAKE CLEAN URL: https://github.com/apache/trafodion/pull/1771#issuecomment-454882627 Hi Kevin, You may be right that this is a fundamental problem of mixing makefiles and maven into single build project. My concern with this change is that it only checks for existence of a file rather than checking it against the dependencies to see if it needs to be re-built. Ideally, makefiles should be written so that if none of the source files have changed, make does not rebuild anything. But if a source file changes, then the right things do get re-built without needing to do make clean. Right now, we have the problem of way too much re-building that does not need to be done. With your change, we have less of that problem, but the developer needs to know what to remove/clean if they do need to re-build something. That may be a better problem to have. It just goes against the ideal makefile methods. Example: Trafci_jar: mvn clean install -DskipTests This rule always runs becuase "Trafci_jar" is not a real file, instead make is asking maven to make sure it is up to date, but maven is told not to trust existing objects by doing clean first. (Ugh). You are changing this to: Trafci_jar: @if [ ! -f target/trafciInstaller.jar ] ; then \ mvn install -DskipTests ; \ fi Which is not running maven at all regardless of dependencies if the jar exists. The simpler way to write that rule is: target/trafci.jar: mvn install -DskipTests (Replacing all references of "Trafci_jar" in the makefile with the full jar file name.) Even better would be something like this: target/trafci.jar: src/main/java/org/trafodion/ci/* mvn install -DskipTests Which would cause a rebuild if any sources in that directory changed. Other dependencies may also be needed, and nice to have. But at least use the simpler makefile syntax instead of shell script in the rule. In this particular case, the rules are even more complicated because there are two different pom files in the same directory that probably interfere with each other. So fixing this is not easy.
---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services
