Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TEST/APPS: Fixed building rpm. #10358

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

rakhmets
Copy link
Collaborator

@rakhmets rakhmets commented Dec 5, 2024

What?

Fixed the following scenario:

$ ./autogen.sh
$ ./contrib/configure-release --with-java=no
$ ./contrib/buildrpm.sh -s -t -b
...
nvcc fatal   : Could not open output file .deps/test_cuda_get_symbol_address.Tpo

@Alexey-Rivkin
Copy link
Collaborator

/azp run UCX release

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rakhmets rakhmets force-pushed the topic/fix-buildrpm-w-cuda branch from 50ce59c to cb3e27c Compare December 5, 2024 18:57
`test -f '$<' || echo '$(srcdir)/'`$<
$(AM_V_at)$(am__mv) $(DEPDIR)/test_cuda_get_symbol_address.Tpo \
$(DEPDIR)/test_cuda_get_symbol_address.Po
@am__fastdepCC_TRUE@ $(AM_V_CC)$(NVCC) $(DEFS) $(DEFAULT_INCLUDES) $(INCLUDES) $(test_cuda_get_symbol_address_CPPFLAGS) -Xcompiler "$(CPPFLAGS) $(CFLAGS)" -MT $@ -MD -MF $(DEPDIR)/test_cuda_get_symbol_address.Tpo -c -o $@ `test -f 'test_cuda_get_symbol_address.cu' || echo '$(srcdir)/'`test_cuda_get_symbol_address.cu
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any way to make this simpler? maybe some kind of intermediate/explicit dependency?
what is the issue being solved here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the same command line that is autogenerated and used to build objects from .c files. The only difference are: $(NVCC) instead of $(CC), -Xcompiler to pass options not supported by nvcc, and the absence of -MP option.

@am__fastdepCC_TRUE@, @AMDEP_TRUE@@am__fastdepCC_FALSE@, @am__fastdepCC_FALSE@ are used to comment out the unused lines depending on the build type.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there some way to avoid taking the autogenerated command and instead just tweak some automake vars?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or at least remove some parts of the command which are not essential?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not find an easier way to use two different command line for different compilers. I can try to simplify the command line a little. However, I do not know any other ways to support different command lines for make/make dist/etc... other than using the same variables to comment out the lines.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, so let's try to simplify it

@rakhmets rakhmets requested a review from yosefe December 19, 2024 17:54
Comment on lines +128 to +132
@am__fastdepCC_TRUE@ $(AM_V_CC)$(test_cuda_get_symbol_address_COMPILE) -MT $@ -MD -MF $(test_cuda_get_symbol_address_DEPBASE).Tpo -c -o $@ `test -f '$(<F)' || echo '$(srcdir)/'`$(<F)
@am__fastdepCC_TRUE@ $(AM_V_at)$(am__mv) $(test_cuda_get_symbol_address_DEPBASE).Tpo $(test_cuda_get_symbol_address_DEPBASE).Po
@AMDEP_TRUE@@am__fastdepCC_FALSE@ $(AM_V_CC)source='$(<F)' object='$@' libtool=no @AMDEPBACKSLASH@
@AMDEP_TRUE@@am__fastdepCC_FALSE@ DEPDIR=$(DEPDIR) $(CCDEPMODE) $(depcomp) @AMDEPBACKSLASH@
@am__fastdepCC_FALSE@ $(AM_V_CC@am__nodep@)$(test_cuda_get_symbol_address_COMPILE) -Xcompiler "-fPIC" -c -o $@ `test -f '$(<F)' || echo '$(srcdir)/'`$(<F)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we simplify this further? for example, do we readlly need am__fastdepCC_TRUE/am__fastdepCC_FALSE ? seems it switches between 2 modes of deps, can we use just one?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, these variables are replaced by # to comment out lines depending on the configure option (dependency-tracking) and the compiler version.
We need these variables at the beginning of the line to compile the object file in the same way as all other object files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants