Page MenuHomeFreeBSD

New port: audio/faust: Functional programming language for realtime audio signal processing
ClosedPublic

Authored by yuri on Nov 1 2017, 8:21 AM.

Details

Summary

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=221509

Patches look a bit scary because their coding style is very loose. They hardcode 'make' and optimization flags in Makefiles a lot, etc.

It also requires a specific version of llvm. Other versions don't work.

Faust, though, implements a new approach to digital sound processing with a special language, compiled into executables of various kinds that are immediately able to convert sound without any third-party libraries.

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

yuri created this revision.Nov 1 2017, 8:21 AM
yuri added a reviewer: adamw.Nov 1 2017, 9:04 AM
tcberner added inline comments.Nov 1 2017, 6:46 PM
audio/faust/Makefile
4 ↗(On Diff #34585)

^ switch to DISTVERSION and reorder with DISTVERSIONPREFIX.

31 ↗(On Diff #34585)

^ missing <tab>?

37 ↗(On Diff #34585)

^ missing <tab>

39 ↗(On Diff #34585)

^ MAKE_ENV ?

audio/faust/pkg-plist
995 ↗(On Diff #34585)

^ can you double check that these manually created directories are properly removed on uninstall?

yuri marked 4 inline comments as done.Nov 1 2017, 9:55 PM
yuri updated this revision to Diff 34641.
mat added a comment.Nov 2 2017, 2:49 PM

And pkg-message is way way too long.

audio/faust/Makefile
14–15 ↗(On Diff #34641)

A dependency on a shared library is better done as a LIB_DEPENDS, in this case I do not think it is possible, so it would probably be easier and faster to write:

llvm${LLVM_VERSION}>0:devel/llvm${LLV_MVERSION}
20 ↗(On Diff #34641)

Remove this and set:

DISTVERSION= 2-1-0

(and reread 5.2.2. Versions, DISTVERSION or PORTVERSION)

23 ↗(On Diff #34641)

This does not do anything.

25 ↗(On Diff #34641)

This does not do anything.

31 ↗(On Diff #34641)

missing tab after =

yuri marked 6 inline comments as done.Nov 4 2017, 2:43 PM
yuri added inline comments.
audio/faust/Makefile
25 ↗(On Diff #34641)

Without WITH_OPENSSL_PORT=yes build fails: you should add the directory containing `libcrypto.pc'

37 ↗(On Diff #34585)

Is at least one tab required? There are a lot of ports that have no tab after = (ex: grep "^[A-Z_]*=[A-Z]" /usr/ports/devel/*/Makefile).

IMO it looks better when the right side of = is aligned. Please let me know if you insist on tab, I will add it.

audio/faust/pkg-plist
995 ↗(On Diff #34585)

Yes, they are properly deleted.

yuri marked an inline comment as done.Nov 4 2017, 2:44 PM
yuri edited the summary of this revision. (Show Details)
yuri updated this revision to Diff 34750.

Fixed requested items.

yuri marked an inline comment as done.Nov 4 2017, 2:45 PM
yuri marked an inline comment as done.
mat requested changes to this revision.Nov 7 2017, 4:13 PM

requesting a change for the WITH_OPENSSL_PORT bit.

audio/faust/Makefile
25 ↗(On Diff #34641)

If this does not build with OpenSSL from the base system, then add:

BROKEN_SSL= base

But do not add WITH_OPENSSL_PORT. It was always supposed to be a end user knob, to be set in /etc/make.conf, not in a per port basis.

37 ↗(On Diff #34585)

At last a tab is required, yes. Just because other poeple did stupid things and they ave not yet been fixed does not mean you can add more.

This revision now requires changes to proceed.Nov 7 2017, 4:13 PM
yuri updated this revision to Diff 34888.Nov 7 2017, 5:22 PM

Requested changes.

yuri marked 3 inline comments as done.Nov 7 2017, 5:24 PM
yuri added inline comments.
audio/faust/Makefile
25 ↗(On Diff #34641)

Removed WITH_OPENSSL_PORT.

yuri marked an inline comment as done.Nov 7 2017, 5:25 PM
yuri marked an inline comment as done.
yuri awarded a token.Nov 11 2017, 8:13 PM
tcberner added inline comments.Nov 11 2017, 9:01 PM
audio/faust/Makefile
32 ↗(On Diff #34889)

^ are you sure, this is only a build-time dependency? otherwise, I would add it as HTTPD_LIB_LIB_DEPENDS=libmicrohttpd.so:www/libmcorhttpd.

yuri marked an inline comment as done.Nov 11 2017, 9:09 PM
yuri added inline comments.
audio/faust/Makefile
32 ↗(On Diff #34889)

I think they use /usr/local/lib/libmicrohttpd.a, which makes it a build dependency.
'make stage-qa' reliably checks for run dependencies, and it doesn't find it. So it isn't a run dependency.

yuri marked 2 inline comments as done.Nov 11 2017, 9:11 PM
tcberner requested changes to this revision.Nov 11 2017, 9:12 PM

It fails to build on 10.3@i386 unfortunately.

rm -f libfaust.a
ar cq libfaust.a ../libraries/loki/SafeFormat.o boxes/boxcomplexity.o boxes/boxes.o boxes/boxtype.o boxes/ppbox.o documentator/doc.o documentator/doc_Text.o documentator/doc_autodoc.o documentator/doc_compile.o documentator/doc_lang.o documentator/doc_metadatas.o documentator/doc_notice.o documentator/doc_sharing.o documentator/lateq.o draw/device/PSDev.o draw/device/SVGDev.o draw/drawschema.o draw/schema/blockSchema.o draw/schema/cableSchema.o draw/schema/collector.o draw/schema/connectorSchema.o draw/schema/cutSchema.o draw/schema/decorateSchema.o draw/schema/enlargedSchema.o draw/schema/inverterSchema.o draw/schema/mergeSchema.o draw/schema/parSchema.o draw/schema/recSchema.o draw/schema/seqSchema.o draw/schema/splitSchema.o draw/schema/topSchema.o draw/sigToGraph.o errors/errormsg.o errors/timing.o evaluate/environment.o evaluate/eval.o evaluate/loopDetector.o extended/xtended.o generator/Text.o generator/code_container.o generator/contextor.o generator/dag_instructions_compiler.o generator/description.o generator/dsp_aux.o generator/export.o generator/fir_to_fir.o generator/floats.o generator/instructions.o generator/instructions_compiler.o generator/occurences.o generator/omp_code_container.o generator/tools.o generator/uitree.o generator/vec_code_container.o generator/wss_code_container.o normalize/aterm.o normalize/mterm.o normalize/normalize.o normalize/privatise.o normalize/simplify.o parallelize/code_loop.o parallelize/colorize.o parser/enrobage.o parser/faustlexer.o parser/faustparser.o parser/sourcefetcher.o parser/sourcereader.o patternmatcher/patternmatcher.o propagate/labels.o propagate/propagate.o signals/binop.o signals/ppsig.o signals/prim2.o signals/recursivness.o signals/signals.o signals/sigorderrules.o signals/sigprint.o signals/sigtype.o signals/sigtyperules.o signals/sigvisitor.o signals/subsignals.o tlib/compatibility.o tlib/list.o tlib/node.o tlib/occurrences.o tlib/recursive-tree.o tlib/shlysis.o tlib/symbol.o tlib/tree.o utils/files.o utils/names.o libcode.o global.o generator/asmjs/asmjs_code_container.o generator/asmjs/asmjs_dsp_aux.o generator/c/c_code_container.o generator/cpp/cpp_code_container.o generator/cpp/cpp_gpu_code_container.o generator/fir/fir_code_container.o generator/interpreter/interpreter_dsp_aux.o generator/interpreter/interpreter_code_container.o generator/java/java_code_container.o generator/js/js_code_container.o generator/llvm/clang_code_container.o generator/llvm/llvm_dsp_aux.o generator/llvm/llvm_code_container.o generator/wasm/wasm_dsp_aux.o generator/wasm/wast_code_container.o generator/wasm/wasm_code_container.o libmain.o
Package libcrypto was not found in the pkg-config search path.
Perhaps you should add the directory containing `libcrypto.pc'
to the PKG_CONFIG_PATH environment variable
Package 'libcrypto', required by 'virtual:world', not found
generator/dsp_aux.o: In function `generateSHA1(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&)':
generator/dsp_aux.cpp:(.text+0x18ae): undefined reference to `SHA1'
c++: error: linker command failed with exit code 1 (use -v to see invocation)
gmake[2]: *** [Makefile.unix:285: faust] Error 1
gmake[2]: *** Waiting for unfinished jobs....
ranlib libfaust.a
gmake[2]: Leaving directory '/wrkdirs/usr/ports/audio/faust/work/faust-2-1-0/compiler'
gmake[1]: *** [Makefile:44: all] Error 2
gmake[1]: Leaving directory '/wrkdirs/usr/ports/audio/faust/work/faust-2-1-0'
===> Compilation failed unexpectedly.
Try to set MAKE_JOBS_UNSAFE=yes and rebuild before reporting the failure to
the maintainer.
*** Error code 1
This revision now requires changes to proceed.Nov 11 2017, 9:12 PM

...also on current@amd64

yuri added a comment.Nov 11 2017, 9:18 PM

It fails to build on 10.3@i386 unfortunately.

This is because I removed WITH_OPENSSL_PORT=yes per mat's request. It worked on 10.X when I've originally created this port.
I will re-add WITH_OPENSSL_PORT.

In D12890#271296, @yuri wrote:

It fails to build on 10.3@i386 unfortunately.

This is because I removed WITH_OPENSSL_PORT=yes per mat's request. It worked on 10.X when I've originally created this port.
I will re-add WITH_OPENSSL_PORT.

That's probably not the best solution.

yuri added a comment.Nov 11 2017, 9:27 PM

It wants .pc file that the base OpenSSL doesn't provide.
Additionally, ports shouldn't use the base OpenSSL at all because of conflicts.

yuri added a comment.Nov 11 2017, 9:27 PM

That's probably not the best solution.

Why?

yuri added a comment.EditedNov 11 2017, 9:29 PM

Base vs. port OpenSSL was discussed in the ports@ ML over a year ago. There were a lot of conflicts. The consensus was that the base OpenSSL shouldn't be used in ports because their mix causes conflicts.

You should add
BROKEN_SSL=base
but not set the knob manually.

You can add a in your poudriere make.conf
DEFAULT_VERSIONS+=ssl=openssl
which in turn will make the port use openssl from ports not base

yuri added a comment.Nov 11 2017, 9:36 PM

You should add
BROKEN_SSL=base
but not set the knob manually.
You can add a in your poudriere make.conf
DEFAULT_VERSIONS+=ssl=openssl
which in turn will make the port use openssl from ports not base

Ok, trying this.

yuri added a comment.EditedNov 11 2017, 9:48 PM
In D12890#271302, @yuri wrote:

You should add
BROKEN_SSL=base
but not set the knob manually.
You can add a in your poudriere make.conf
DEFAULT_VERSIONS+=ssl=openssl
which in turn will make the port use openssl from ports not base

Ok, trying this.

BROKEN_SSL=base alone isn't sufficient. It breaks on 11.1. But it passes on 11,1 with WITH_OPENSSL_PORT=yes.

In D12890#271303, @yuri wrote:
In D12890#271302, @yuri wrote:

You should add
BROKEN_SSL=base
but not set the knob manually.
You can add a in your poudriere make.conf
DEFAULT_VERSIONS+=ssl=openssl
which in turn will make the port use openssl from ports not base

Ok, trying this.

'''BROKEN_SSL=base''' alone isn't sufficient. It breaks on 11.1. But it passes on '''WITH_OPENSSL_PORT=yes'''.

BROKEN_SSL=base means that the port will be marked broken if the ssl-default version is not overriden.
But unlike forcing WITH_OPENSSL_PORT it means it will also allow for building with libressl, should that one be set as ssl-default version.

You can read more on it in

  • Mk/Uses/ssl.mk
  • Mk/bsd.default-versions.mk

For example I have now for my 120amd64-jail with portstree yuri the following make.conf
/usr/local/etc/poudriere.d/120amd64-yuri-make.conf:

DEFAULT_VERSIONS+=ssl=openssl

Which means the ports will be built against security/openssl. Including audio/faust -- so it builds fine.

If it is however not set (or set to base) it should be marked as broken.
If that is not the case, then something else is not good :D

yuri updated this revision to Diff 35140.Nov 12 2017, 7:40 AM

Updated openssl dependency.

yuri added a comment.EditedNov 12 2017, 7:44 AM
DEFAULT_VERSIONS+=ssl=openssl

I couldn't see that BROKEN_SSL=base labels it BROKEN. Instead, it just runs into the build and fails there for missing .pc file.
But DEFAULT_VERSIONS+=ssl=openssl in make.conf makes it succeed.
So it will only work when when this instruction is present, as opposed to WITH_OPENSSL_PORT=yes making it always work.

I updated it, let's commit it this way.

yuri added a comment.Nov 12 2017, 9:01 PM

@tcberner

This port is ready IMO. Two problems both aren't related to this port:

  1. portlint error about BROKEN_SSL having to appear earlier - this is a bug in portlint. I will report it once committed.
  2. BROKEN_SSL doesn't really cause it being considered broken. This seems to be a bug in the framework. I think you said that you've let mat@ know.
In D12890#271617, @yuri wrote:

@tcberner
This port is ready IMO. Two problems both aren't related to this port:

  1. portlint error about BROKEN_SSL having to appear earlier - this is a bug in portlint. I will report it once committed.
  2. BROKEN_SSL doesn't really cause it being considered broken. This seems to be a bug in the framework. I think you said that you've let mat@ know.

I think so, but I would like some feedback from mat first, to be sure.

mat added inline comments.Nov 24 2017, 5:02 PM
audio/faust/Makefile
32 ↗(On Diff #34889)

One should never use static libraries, it makes it a mess to know what needs to be rebuilt, and reinstalled when security issues happen.

yuri updated this revision to Diff 36792.Dec 20 2017, 4:56 AM

Update: now SSL is *not* required any more (in this new version), so most of the previous notes become moot.

yuri added a comment.Dec 20 2017, 4:59 AM

@adamw This port is also ready.

The previous version required SSL, and it was conflicting with DEFAULT_VERSIONS= ssl=base.
Upstream dropped SSL, so now it is free of this contention.
I will also submit all patches upstream as pull requests.

yuri updated this revision to Diff 36793.Dec 20 2017, 5:06 AM

Made microhttpd LIB_DEPENDS.

yuri marked an inline comment as done.Dec 20 2017, 5:06 AM
adamw added inline comments.Dec 20 2017, 5:30 AM
audio/faust/Makefile
24 ↗(On Diff #36794)

This is an extremely long way to say "3.4". Why programmatically derive a string when there's only one possible value?

yuri marked an inline comment as done.Dec 20 2017, 5:33 AM
yuri added inline comments.
audio/faust/Makefile
24 ↗(On Diff #36794)

The idea is that only one variable holds the llvm version: LLVM_VERSION, so that it isn't necessary to retype in many places, which is error-prone.

adamw added inline comments.Dec 20 2017, 5:39 AM
audio/faust/Makefile
24 ↗(On Diff #36794)

Sure, but it only has one possible value. If the port ever supports more than one llvm version, you won't be able to use LLVM_VERSION anyway.

It's not a big deal, and if you're happier with it how it is then I'm fine with it. I just personally think that clever is more error-prone than clear.

yuri marked 3 inline comments as done.Dec 20 2017, 6:19 AM
yuri added inline comments.
audio/faust/Makefile
24 ↗(On Diff #36794)

I would like to update it to some later version.
It should be the latest llvm, but no other version worked when I tried.

I would prefer to keep the version in one place. The llvm version is always 2 digits, and it it's likely to stay this way until clang-10 in many years. :)

adamw accepted this revision.Dec 20 2017, 7:44 PM

Well, as long as you've verified that it builds successfully everywhere, go for it.

One thing you might consider changing when you commit: if you set
REINPLACE_ARGS= -i ''
then you don't need to keep specifying it over and over.

yuri marked an inline comment as done.Dec 20 2017, 9:07 PM
This revision was not accepted when it landed; it landed in state Needs Review.
This revision was automatically updated to reflect the committed changes.