ComposersDesktop / CDP8

New version of CDP software
GNU Lesser General Public License v2.1
90 stars 3 forks source link

Nix derivation #2

Open ovitus opened 11 months ago

ovitus commented 11 months ago

I'm getting this error during compile:

[  0%] Building C object dev/cdp2k/CMakeFiles/cdp2k.dir/__/cdparse/cdparse.c.o
[  0%] Building C object dev/cdp2k/CMakeFiles/cdp2k.dir/dzsetup.c.o
[  0%] Building C object dev/cdp2k/CMakeFiles/cdp2k.dir/formantsg.c.o
[  1%] Building C object dev/cdp2k/CMakeFiles/cdp2k.dir/mainfuncs.c.o
[  1%] Building C object dev/cdp2k/CMakeFiles/cdp2k.dir/parstruct.c.o
[  1%] Building C object dev/cdp2k/CMakeFiles/cdp2k.dir/readdata.c.o
[  1%] Building C object dev/cdp2k/CMakeFiles/cdp2k.dir/readfiles.c.o
[  1%] Building C object dev/cdp2k/CMakeFiles/cdp2k.dir/special.c.o
[  1%] Building C object dev/cdp2k/CMakeFiles/cdp2k.dir/tkinput.c.o
[  2%] Building C object dev/cdp2k/CMakeFiles/cdp2k.dir/tklib1.c.o
[  2%] Building C object dev/cdp2k/CMakeFiles/cdp2k.dir/tklib3.c.o
[  2%] Building C object dev/cdp2k/CMakeFiles/cdp2k.dir/validate.c.o
[  2%] Building C object dev/cdp2k/CMakeFiles/cdp2k.dir/writedata.c.o
[  2%] Linking C static library libcdp2k.a             
[  2%] Built target cdp2k  
[  2%] Building C object dev/newsfsys/CMakeFiles/sfsys.dir/sfsys.c.o
[  2%] Building C object dev/newsfsys/CMakeFiles/sfsys.dir/snd.c.o                                                                                             
[  3%] Building C object dev/newsfsys/CMakeFiles/sfsys.dir/sfdir.c.o
[  3%] Building C object dev/newsfsys/CMakeFiles/sfsys.dir/osbind.c.o         
[  3%] Building C object dev/newsfsys/CMakeFiles/sfsys.dir/props.c.o
[  3%] Building C object dev/newsfsys/CMakeFiles/sfsys.dir/ieee80.c.o
[  3%] Building C object dev/newsfsys/CMakeFiles/sfsys.dir/pvfileio.c.o
[  3%] Linking C static library libsfsys.a                           
[  3%] Built target sfsys                                                      
[  3%] Building C object dev/pvxio2/CMakeFiles/pvxio2.dir/pvfileio.c.o
In function ‘prepare_pvfmt’,                                                   
    inlined from ‘pvoc_createfile’ at /home/awright/CDP8/dev/pvxio2/pvfileio.c:458:5:
/home/awright/CDP8/dev/pvxio2/pvfileio.c:378:35: warning: ‘*(WAVEFORMATEX *)pfile.nBlockAlign’ may be used uninitialized [-Wmaybe-uninitialized]
  378 |     pfmt->nAvgBytesPerSec   = pfmt->nBlockAlign * srate;  
      |                               ~~~~^~~~~~~~~~~~~             
[  4%] Linking C static library libpvxio2.a                          
[  4%] Built target pvxio2                                                     
[  4%] Building C object dev/blur/CMakeFiles/blur.dir/main.c.o
[  4%] Building C object dev/blur/CMakeFiles/blur.dir/ap_blur.c.o
[  5%] Building C object dev/blur/CMakeFiles/blur.dir/blur.c.o      
[  5%] Linking C executable ../../NewRelease/blur                                                                                                              
/usr/bin/ld: ../cdp2k/libcdp2k.a(cdparse.c.o):(.bss+0x0): multiple definition of `errstr'; CMakeFiles/blur.dir/main.c.o:(.bss+0x20): first defined here
collect2: error: ld returned 1 exit status
make[2]: *** [dev/blur/CMakeFiles/blur.dir/build.make:132: NewRelease/blur] Error 1
make[1]: *** [CMakeFiles/Makefile2:1529: dev/blur/CMakeFiles/blur.dir/all] Error 2
make: *** [Makefile:156: all] Error 2   

Happens with GCC and Clang on NixOS, Debian and RHEL. Any idea on how to fix this?

richarddobson commented 11 months ago

A missing #ifdef, should have been there long ago, fixed in git. Good catch, thanks! RWD.

PS: should you have problems building the audio programs (pvplay etc), make sure you have built and fully installed the current stable version of portaudio, not a weekly/daily snapshot.

On 18/10/2023 02:22, Andrew Garrett Wright wrote: ..

../cdp2k/libcdp2k.a(cdparse.c.o):(.bss+0x0): multiple definition of `errstr'; CMakeFiles/blur.dir/main.c.o:(.bss+0x20): first defined here ..

ovitus commented 11 months ago

I got a bit further but can't figure this one out:

[ 99%] Linking C executable /nix/store/mpjrkqajspbx99a9pmbndsg6igfzm6y4-CDP-8.0.1/CDP8/NewRelease/paplay
/nix/store/zkjq96ik8cbv6ijh1lylnkk2bni9qvas-binutils-2.40/bin/ld: CMakeFiles/paplay.dir/paplay.c.o: in function `main':
paplay.c:(.text+0xc38): undefined reference to `PaUtil_InitializeRingBuffer'
/nix/store/zkjq96ik8cbv6ijh1lylnkk2bni9qvas-binutils-2.40/bin/ld: CMakeFiles/paplay.dir/paplay.c.o: in function `paplayCallback':
paplay.c:(.text+0x17c5): undefined reference to `PaUtil_GetRingBufferReadAvailable'
/nix/store/zkjq96ik8cbv6ijh1lylnkk2bni9qvas-binutils-2.40/bin/ld: paplay.c:(.text+0x1820): undefined reference to `PaUtil_ReadRingBuffer'
/nix/store/zkjq96ik8cbv6ijh1lylnkk2bni9qvas-binutils-2.40/bin/ld: CMakeFiles/paplay.dir/paplay.c.o: in function `threadFunctionReadFromRawFile':
paplay.c:(.text+0x1d0e): undefined reference to `PaUtil_AdvanceRingBufferWriteIndex'
/nix/store/zkjq96ik8cbv6ijh1lylnkk2bni9qvas-binutils-2.40/bin/ld: paplay.c:(.text+0x1d20): undefined reference to `PaUtil_GetRingBufferWriteAvailable'
/nix/store/zkjq96ik8cbv6ijh1lylnkk2bni9qvas-binutils-2.40/bin/ld: paplay.c:(.text+0x1d69): undefined reference to `PaUtil_GetRingBufferWriteRegions'
clang-11: error: linker command failed with exit code 1 (use -v to see invocation)
make[2]: *** [dev/externals/paprogs/paplay/CMakeFiles/paplay.dir/build.make:116: /nix/store/mpjrkqajspbx99a9pmbndsg6igfzm6y4-CDP-8.0.1/CDP8/NewRelease/paplay] Error 1
make[1]: *** [CMakeFiles/Makefile2:7671: dev/externals/paprogs/paplay/CMakeFiles/paplay.dir/all] Error 2
make: *** [Makefile:156: all] Error 2
error: builder for '/nix/store/djxc3rc84180nzkl7ynn8p93yqsp493d-CDP-8.0.1.drv' failed with exit code 2;
       last 10 log lines:
       > paplay.c:(.text+0x17c5): undefined reference to `PaUtil_GetRingBufferReadAvailable'
       > /nix/store/zkjq96ik8cbv6ijh1lylnkk2bni9qvas-binutils-2.40/bin/ld: paplay.c:(.text+0x1820): undefined reference to `PaUtil_ReadRingBuffer'
       > /nix/store/zkjq96ik8cbv6ijh1lylnkk2bni9qvas-binutils-2.40/bin/ld: CMakeFiles/paplay.dir/paplay.c.o: in function `threadFunctionReadFromRawFile':
       > paplay.c:(.text+0x1d0e): undefined reference to `PaUtil_AdvanceRingBufferWriteIndex'
       > /nix/store/zkjq96ik8cbv6ijh1lylnkk2bni9qvas-binutils-2.40/bin/ld: paplay.c:(.text+0x1d20): undefined reference to `PaUtil_GetRingBufferWriteAvailable'
       > /nix/store/zkjq96ik8cbv6ijh1lylnkk2bni9qvas-binutils-2.40/bin/ld: paplay.c:(.text+0x1d69): undefined reference to `PaUtil_GetRingBufferWriteRegions'
       > clang-11: error: linker command failed with exit code 1 (use -v to see invocation)
       > make[2]: *** [dev/externals/paprogs/paplay/CMakeFiles/paplay.dir/build.make:116: /nix/store/mpjrkqajspbx99a9pmbndsg6igfzm6y4-CDP-8.0.1/CDP8/NewRelease/paplay] Error 1
       > make[1]: *** [CMakeFiles/Makefile2:7671: dev/externals/paprogs/paplay/CMakeFiles/paplay.dir/all] Error 2
       > make: *** [Makefile:156: all] Error 2
       For full logs, run 'nix log /nix/store/djxc3rc84180nzkl7ynn8p93yqsp493d-CDP-8.0.1.drv'.

I'm using PortAudio v19.7.0 from github, I also tried with the tarball. You can see the build commands in the repo I have for the nix derivation, here.

ovitus commented 11 months ago

Got it to compile. From pa_ringbuffer.h:

@note The ring buffer functions are not normally exposed in the PortAudio libraries. If you want to call them then you will need to add pa_ringbuffer.c to your application source code.

In case anybody is wondering how exactly: I appended pa_ringbuffer.c to add_executable in: /home/wright/CDP8/dev/externals/paprogs/{paplay,pvplay,recsf}/CMakeLists.txt and soft linked the source file from PortAudio's src/common to those directories.

Maybe there's a way to integrate this so I don't need a separate fork? I don't know how other people got this to compile without this fix since those functions are missing.

richarddobson commented 11 months ago

I found that this depended on which code set of portaudio one is using. You will see, for example, in the CMakeLists.txt file for pvplay, that that file is present as a reminder but commented out. When the "stable" version of portaudio (built using ./configure, not Cmake, and which also builds all the pa test programs) is used, as directly downloaded from the portaudio site, with a full 'make install', the explicit addition of pa_ringbuffer.c was not needed. Another user found that using that archive did solve all the portaudio linking problems. I suspect this may be an adjustment that can't be made in the CMakeLists file that will cover all builds of portaudio. It may simply have to be documented as something to check. In any case, we are always warned that the active git repository for pa may include code that is "unsafe" one way or another, and users should normally just use the packaged "stable" version.

On 24/10/2023 02:50, Andrew Garrett Wright wrote:

Got it to compile. From pa_ringbuffer.h:

@note <https://github.com/note> The ring buffer functions are not
normally exposed in the PortAudio libraries.
If you want to call them then you will need to add pa_ringbuffer.c
to your application source code.

In case anybody is wondering how exactly: I appended |pa_ringbuffer.c| to |add_executable| in: |/home/wright/CDP8/dev/externals/paprogs/{paplay,pvplay,recsf}/CMakeLists.txt| and soft linked the source file from PortAudio's |src/common| to those directories.

Maybe there's a way to integrate this so I don't need a separate fork? I don't know how other people got this to compile without this fix since those functions are missing.

— Reply to this email directly, view it on GitHub https://github.com/ComposersDesktop/CDP8/issues/2#issuecomment-1776348080, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABNHS4W2M456UNUXO2JV5W3YA4NF5AVCNFSM6AAAAAA6EWOIMSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONZWGM2DQMBYGA. You are receiving this because you commented.Message ID: @.***>

ovitus commented 11 months ago

Are these the PA test programs you're referring to? Is there an option to explicitly build these? My PA derivation does install with ./configure and "make install". Since Nix hashes and compartmentalizes everything in /nix/store and not /usr/local it's possible some of the directories are not being seen as in a traditional install. I will test again on my Debian system with the website tarball of PA. Thanks you!

richarddobson commented 11 months ago

Yes. They are quite useful in lots of ways. When you run ./configure on portaudio, it should print a line to show that the test programs will be built, which IIRC is the default. Otherwise, there will be a setting you can change.

I can't comment on your Nix system, never used it. If you want to write up how to use that, and maybe make any changes in git that don't break the current "traditional" setup, that would clearly be a valuable addition. But the CMakeLists.txt files for the audio programs do expect to find libportaudio.a in /usr/local/lib. If they are elsewhere, you would have to make private changes to tell Cmake the required paths - or just manually copy them to /usr/local/lib.

I had assumed you were building for the Mac, but are you in fact building for Linux? or both? That is more John fitch's area (he created the whole Cmake mechanism, and only builds for linux). I will check to see if he sees these messages.

On 24/10/2023 14:42, Andrew Garrett Wright wrote:

Are these https://github.com/PortAudio/portaudio/tree/24c8d575e588d557d28f4011becb753421346860/test the PA test programs you're referring to? Is there an option to explicitly build these? My PA derivation does install with ./configure and "make install". Since Nix hashes and compartmentalizes everything in /nix/store and not /usr/local it's possible some of the directories are not being seen as in a traditional install. I will test again on my Debian system with the website tarball of PA. Thanks you!

— Reply to this email directly, view it on GitHub https://github.com/ComposersDesktop/CDP8/issues/2#issuecomment-1777232976, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABNHS4VX7FFSVJ2GIDL4XEDYA7AVNAVCNFSM6AAAAAA6EWOIMSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONZXGIZTEOJXGY. You are receiving this because you commented.Message ID: @.***>

ovitus commented 11 months ago

Ok, I'll take another look at compiling PortAudio. Maybe there is a cmake flag to provide the source file without having to edit CMakeLists.txt, I'll have to do more research as I don't know the language or build tools well. I'm primarily on Debian and have been motivated to switch over to NixOS because of the reproducibility. I'm still in the learning phase, this is my first derivation. (:

richarddobson commented 11 months ago

I have now looked up NixOS to see what it is in detail. It looks like it may need a custom set of rules in the CDP CMakeLists.txt files, for the audio programs, to provided the required paths etc. Clearly it has worked OK for building the CDP programs, but has difficulties with portaudio which I suspect does not know about it. It may well be, therefore, that pa_rignbuffer.c does need to be added, under a dedicated section in CMakeLists.txt for NixOS. The question will then be, what OS/Architecture symbols does NixOS perovide via CMake, since "linux" probably is not sufficient in this case. This need not require a fork (no requirement to change the source files themselves), just updates to the current CMakeLists.txt files, using whatever distinguishing symbol is available.

On 26/10/2023 01:40, Andrew Garrett Wright wrote:

Ok, I'll take another look at compiling PortAudio. Maybe there is a cmake flag to provide the source file without having to edit CMakeLists.txt, I'll have to do more research as I don't know the language or build tools well. I'm primarily on Debian and have been motivated to switch over to NixOS because of the reproducibility. I'm still in the learning phase, this is my first derivation. (:

— Reply to this email directly, view it on GitHub https://github.com/ComposersDesktop/CDP8/issues/2#issuecomment-1780243562, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABNHS4VRHLHUSRQISQ4JOZ3YBGWOPAVCNFSM6AAAAAA6EWOIMSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOBQGI2DGNJWGI. You are receiving this because you commented.Message ID: @.***>

ovitus commented 11 months ago

I think if you're willing to add that source file directly into the CDP8 repo, it might be used for all Unix-based compiles and so there wouldn't be a specific NixOS condition for it. Nix is a general purpose package manager that makes use of the aforementioned nix store to keep all of its dependencies discrete and immutable, it's system agnostic and can be used to easily reproduce the build for any system. That said, I can understand somebody not wanting to install another package manager they're unfamiliar with. The question is would including the source file locally as I've done break anything with these more traditional installs? I'd be happy to test it out if you think that might be a viable option.

ovitus commented 11 months ago

I did some further testing on Debian and RHEL with and without Nix. It seems that traditional builds complete without requiring any modification to CMakeLists.txt involving pa_ringbuffer.c. I did however need to copy pa_ringbuffer.h and pa_util.h from PA's src/common to CDP8's dev/include directory. I tried also compiling without Nix with the changes I made and the only difference was that it asked for pa_memorybarrier.h. I copied that over from src/common and the compilation went through.

I don't know enough to understand why the Nix build gets those undefined reference errors when PA is outside of /usr/local/lib. Would providing pa_ringbuffer.c in the CDP8 repo be much of a hassle? Is it usual that I need to copy the header files over to the dev/include directory? Also, I'm grabbing libaaio from CDP7 but shouldn't it be included in CDP8 since it's a dependency, or is there somewhere else to source it?

Just trying to make the build process easier for myself and others. Thanks again

ovitus commented 11 months ago

I decided to skip the 3 programs requiring PA, one less dependency to fuss about. I just used sed in the build commands to change the option and uncomment the conditional. The Nix derivation is here.