Removing extra files copy

Hi

When importing openmp cmake and doing a parallel multi-target
(32/64bit) build - there is a race condition when copying some files.

When looking into this problem it looks like this could or should be
fixed by either removing the extraneous copies (patch below) or by
moving them to a make install target.

https://github.com/pathscale/openmp/commit/6ef0dfb30b1d5475cafb580a29abf73b0dbb6b59

If nobody objects - can someone review and push this.

Thanks

Please can you explain the problem in more detail.
What is actually going wrong?
"a race condition" isn't very explicit.

-- Jim

James Cownie <james.h.cownie@intel.com>
SSG/DPD/TCAR (Technical Computing, Analyzers and Runtimes)
Tel: +44 117 9071438

Exact error (which varies depending on which target causes the race)

Error copying file
"/root/bamboo-agent-home/xml-data/build-dir/DEBUG/build/openmp-llvm/build-x86_32/iomp_lib.h"
to "/root/bamboo-agent-home/xml-data/build-dir/DEBUG/src/openmp-llvm/runtime/exports/lin_32e.deb/include_compat/iomp_lib.h".

To try to explain again - If you are building openmp from a parent
cmake and that cmake is looping over multiple targets you hit the
problem above. (multiple times that file being copied to the same
place)

That file isn't used during the build process at all and I don't know
why it's being copied during the build. Like I mentioned - this should
be removed or moved to an install target.

OK, but your patch removes other copies that are moving target specific things, like the runtime library that was just built.

Here...
-simple_copy_recipe("${lib_file}" "${build_dir}" "${export_lib_dir}")
-simple_copy_recipe("${imp_file}" "${build_dir}" "${export_lib_dir}")
-simple_copy_recipe("${pdb_file}" "${build_dir}" "${export_lib_dir}")
-simple_copy_recipe("${dbg_file}" "${build_dir}" "${export_lib_dir}")

What's happening here isn't really an "install" target, since that normally moves things to wherever they need to be in the machine's file system so that everyone can see them. What's happening here is that we're moving the necessary files to somewhere so that they can be packaged into a distribution.

If you remove these copy operations then you won't be able easily to package the result.

So, I'm nervous about taking this patch as is, since it seems to remove functionality that is required.

I would argue that the root of the problem is that you shouldn’t be building multiple architectures into the same target directory at the same time, not that the cmake file is copying files that the user will need into that directory.

-- Jim

James Cownie <james.h.cownie@intel.com>
SSG/DPD/TCAR (Technical Computing, Analyzers and Runtimes)
Tel: +44 117 9071438

Your argument doesn't stand at all. "moving stuff around" - What's the
point of that? This isn't used in the build process at all and you
just said it's about distribution (That's install target).

Please hide the install stuff under and install target or nuke it from
the build target. This stuff is fugly..

btw - from a packager perspective - if you're building an RPM or deb -
the location to which files are copied from can easily be changed. If
there's a specific packager who doesn't know how to do this this I'm
happy to help them.

The error is a little peculiar because it looks as though the build directory is for 32bit (build-x86_32/), but the destination directory (lin_32e.deb/include_compat/) is 64bit. I'm assuming you have a build-x86_64/ build directory as well that is built in parallel and is also trying to copy iomp_lib.h (or one of the other headers) at the same time to lin_32e.deb/include*? From what I can tell, it should be trying to copy build-x86_32/iomp_lib.h to lin_32.deb/include_compat/ instead of lin_32e.deb/include_compat/ (notice the missing 'e').

Are all the files in build-x86_32/ being copied to lin_32e.deb/*? If that's the case, then the architecture is not being set correctly when calling cmake inside the build-x86_32/ directory. This can be checked by looking at "/root/bamboo-agent-home/xml-data/build-dir/DEBUG/build/openmp-llvm/build-x86_32/CMakeCache.txt and finding the entry for "arch:STRING=..."

-- Johnny