Skip to content

Commit 64eb1c6

Browse files
kornilova203kornilova203
authored
Merge pull request #144 from kornilova-l/error-handling
Improve tooling and sbt-plugin error handling
2 parents c2845fb + 2a3b854 commit 64eb1c6

File tree

5 files changed

+85
-56
lines changed

5 files changed

+85
-56
lines changed

‎sbt-scala-native-bindgen/src/main/scala/org/scalanative/bindgen/sbt/ScalaNativeBindgenPlugin.scala

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -129,20 +129,29 @@ object ScalaNativeBindgenPlugin extends AutoPlugin {
129129
val bindgenPath = nativeBindgenPath.value
130130
val bindings = nativeBindings.value
131131
val outputDirectory = (target in nativeBindgen).value
132+
val logger = streams.value.log
132133

133134
bindings.map {
134135
binding =>
135136
val output = outputDirectory / s"${binding.name}.scala"
136-
137-
Bindgen()
137+
val result = Bindgen()
138138
.bindgenExecutable(bindgenPath)
139139
.header(binding.header)
140140
.name(binding.name)
141141
.maybe(binding.link, _.link)
142142
.maybe(binding.packageName, _.packageName)
143143
.maybe(binding.excludePrefix, _.excludePrefix)
144144
.generate()
145-
.writeToFile(output)
145+
146+
result match {
147+
case Right(bindings) =>
148+
bindings.writeToFile(output)
149+
bindings.errors.foreach(error => logger.error(error))
150+
case Left(errors) =>
151+
errors.foreach(error => logger.error(error))
152+
sys.error(
153+
"scala-native-bindgen failed with non-zero exit code")
154+
}
146155

147156
output
148157
}

‎tests/src/test/scala/org/scalanative/bindgen/BindgenReportingSpec.scala

Lines changed: 42 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,12 @@ class BindgenReportingSpec extends FunSpec {
1919
}
2020
}
2121

22-
def bindgen(input: String): Bindings = {
22+
def assertBindgenError(input: String, errors: Seq[String]): Unit = {
2323
val tempFile = File.createTempFile("scala-native-bindgen-tests", ".h")
2424
try {
2525
writeToFile(tempFile, input)
2626

27-
Bindgen()
27+
val result = Bindgen()
2828
.bindgenExecutable(new File(bindgenPath))
2929
.header(tempFile)
3030
.name("BindgenTests")
@@ -33,51 +33,59 @@ class BindgenReportingSpec extends FunSpec {
3333
.excludePrefix("__")
3434
.generate()
3535

36+
result match {
37+
case Right(binding) =>
38+
assert(binding.errors == errors)
39+
case Left(errors) =>
40+
fail(s"Non-zero exit code:\n${errors.mkString("\n")}")
41+
}
3642
} finally {
3743
tempFile.delete()
3844
}
3945
}
4046

4147
it("Skips functions that pass struct or union by value") {
42-
val bindings =
43-
bindgen(input = """struct s { int a; };
44-
|void useStruct(struct s);
45-
|typedef struct s s;
46-
|s returnStruct();
47-
|
48-
|union u { int a; };
49-
|void useUnion(union u);
50-
|typedef union u u;
51-
|u returnUnion();
52-
|""".stripMargin)
53-
assert(
54-
bindings.errs ==
55-
"""Warning: Function useStruct is skipped because Scala Native does not support passing structs and arrays by value.
56-
|Warning: Function returnStruct is skipped because Scala Native does not support passing structs and arrays by value.
57-
|Warning: Function useUnion is skipped because Scala Native does not support passing structs and arrays by value.
58-
|Warning: Function returnUnion is skipped because Scala Native does not support passing structs and arrays by value.""".stripMargin
48+
assertBindgenError(
49+
"""struct s { int a; };
50+
|void useStruct(struct s);
51+
|typedef struct s s;
52+
|s returnStruct();
53+
|
54+
|union u { int a; };
55+
|void useUnion(union u);
56+
|typedef union u u;
57+
|u returnUnion();
58+
|""".stripMargin,
59+
Seq(
60+
"Warning: Function useStruct is skipped because Scala Native does not support passing structs and arrays by value.",
61+
"Warning: Function returnStruct is skipped because Scala Native does not support passing structs and arrays by value.",
62+
"Warning: Function useUnion is skipped because Scala Native does not support passing structs and arrays by value.",
63+
"Warning: Function returnUnion is skipped because Scala Native does not support passing structs and arrays by value."
64+
)
5965
)
6066
}
6167

6268
it("Skips variable with opaque type") {
63-
val bindings =
64-
bindgen(input = """struct undefinedStruct;
65-
|extern struct undefinedStruct removedExtern;
66-
|#define removedExternAlias removedExtern
67-
|""".stripMargin)
68-
assert(
69-
bindings.errs == """Error: Variable removedExtern is skipped because it has incomplete type.
70-
|Error: Variable alias removedExternAlias is skipped because it has incomplete type.""".stripMargin)
71-
69+
assertBindgenError(
70+
"""struct undefinedStruct;
71+
|extern struct undefinedStruct removedExtern;
72+
|#define removedExternAlias removedExtern
73+
|""".stripMargin,
74+
Seq(
75+
"Error: Variable removedExtern is skipped because it has incomplete type.",
76+
"Error: Variable alias removedExternAlias is skipped because it has incomplete type."
77+
)
78+
)
7279
}
7380

7481
it("Skips unused alias for opaque type") {
75-
val bindings =
76-
bindgen(input = """union undefinedUnion;
77-
|typedef union undefinedUnion aliasForUndefinedUnion;
78-
|""".stripMargin)
79-
assert(
80-
bindings.errs == "Warning: type alias aliasForUndefinedUnion is skipped because it is an unused alias for incomplete type.")
82+
assertBindgenError(
83+
"""union undefinedUnion;
84+
|typedef union undefinedUnion aliasForUndefinedUnion;
85+
|""".stripMargin,
86+
Seq(
87+
"Warning: type alias aliasForUndefinedUnion is skipped because it is an unused alias for incomplete type.")
88+
)
8189
}
8290
}
8391
}

‎tests/src/test/scala/org/scalanative/bindgen/BindgenSpec.scala

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,21 @@ class BindgenSpec extends FunSpec {
1818
}
1919

2020
def bindgen(inputFile: File, name: String, outputFile: File): Unit = {
21-
Bindgen()
21+
val result = Bindgen()
2222
.bindgenExecutable(new File(bindgenPath))
2323
.header(inputFile)
2424
.name(name)
2525
.link("bindgentests")
2626
.packageName("org.scalanative.bindgen.samples")
2727
.excludePrefix("__")
2828
.generate()
29-
.writeToFile(outputFile)
29+
30+
result match {
31+
case Right(binding) =>
32+
binding.writeToFile(outputFile)
33+
case Left(errors) =>
34+
fail("scala-native-bindgen failed: " + errors.mkString("\n"))
35+
}
3036
}
3137

3238
def contentOf(file: File) =

‎tools/src/main/scala/org/scalanative/bindgen/Bindgen.scala

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package org.scalanative.bindgen
33
import java.io.File
44

55
import scala.collection.immutable.Seq
6+
import scala.collection.mutable.ListBuffer
67
import scala.sys.process.{Process, ProcessLogger}
78

89
sealed trait Bindgen {
@@ -51,8 +52,9 @@ sealed trait Bindgen {
5152

5253
/**
5354
* Run binding generator
55+
* @return errors if exit code was not 0, otherwise return bindings
5456
*/
55-
def generate(): Bindings
57+
def generate(): Either[Seq[String], Bindings]
5658
}
5759

5860
object Bindgen {
@@ -69,46 +71,47 @@ object Bindgen {
6971
extends Bindgen {
7072

7173
def bindgenExecutable(executable: File): Bindgen = {
72-
require(executable.exists())
74+
require(executable.exists(), s"Executable does not exist: $executable")
7375
copy(executable = Option(executable))
7476
}
7577

7678
def header(header: File): Bindgen = {
77-
require(header.exists())
79+
require(header.exists(), s"Header file does not exist: $header")
7880
copy(header = Option(header))
7981
}
8082

8183
def link(library: String): Bindgen = {
82-
require(!library.isEmpty)
84+
require(library.nonEmpty, "Library must be non-empty")
8385
copy(library = Option(library))
8486
}
8587

8688
def name(name: String): Bindgen = {
87-
require(!name.isEmpty)
89+
require(name.nonEmpty, "Name must be non-empty")
8890
copy(name = Option(name))
8991
}
9092

9193
def packageName(packageName: String): Bindgen = {
92-
require(!packageName.isEmpty)
94+
require(packageName.nonEmpty, "Package name must be non-empty")
9395
copy(packageName = Option(packageName))
9496
}
9597

9698
def excludePrefix(prefix: String): Bindgen = {
97-
require(!prefix.isEmpty)
99+
require(prefix.nonEmpty, "Exclude prefix must be non-empty")
98100
copy(excludePrefix = Option(prefix))
99101
}
100102

101103
def extraArg(args: String*): Bindgen = {
102-
require(args.forall(_.nonEmpty))
104+
require(args.forall(_.nonEmpty), "All extra-args must be non-empty")
103105
copy(extraArg = extraArg ++ args)
104106
}
105107

106108
def extraArgBefore(args: String*): Bindgen = {
107-
require(args.forall(_.nonEmpty))
109+
require(args.forall(_.nonEmpty),
110+
"All extra-args-before must be non-empty")
108111
copy(extraArgBefore = extraArgBefore ++ args)
109112
}
110113

111-
def generate(): Bindings = {
114+
def generate(): Either[Seq[String], Bindings] = {
112115
require(executable.isDefined, "The executable must be specified")
113116
require(header.isDefined, "Header file must be specified")
114117

@@ -129,13 +132,15 @@ object Bindgen {
129132
withArgs("--extra-arg-before", extraArgBefore) ++
130133
Seq(header.get.getAbsolutePath, "--")
131134

132-
var errs = Seq[String]()
135+
val stdout = ListBuffer[String]()
136+
val stderr = ListBuffer[String]()
137+
val nl = System.lineSeparator()
138+
val logger = ProcessLogger(stdout.+=, stderr.+=)
133139

134-
val output = Process(cmd).!!(ProcessLogger { err: String =>
135-
errs :+= err
136-
})
137-
138-
new Bindings(output, errs.mkString("\n"))
140+
Process(cmd).!(logger) match {
141+
case 0 => Right(new Bindings(stdout.mkString(nl), Seq(stderr: _*)))
142+
case _ => Left(Seq(stderr: _*))
143+
}
139144
}
140145
}
141146
}

‎tools/src/main/scala/org/scalanative/bindgen/Bindings.scala

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
package org.scalanative.bindgen
22

33
import java.io.{File, PrintWriter}
4+
import scala.collection.immutable.Seq
45

5-
class Bindings(private val bindings: String, val errs: String) {
6+
class Bindings(private val bindings: String, val errors: Seq[String]) {
67
def writeToFile(file: File): Unit = {
78
file.getParentFile.mkdirs()
89
new PrintWriter(file) {

0 commit comments

Comments
 (0)