Skip to content
This repository

Warning vs Fatal Error for packages that can't be built #93

Open
wants to merge 1 commit into from

2 participants

Louis Gesbert Fabrice Le Fessant
Louis Gesbert
Collaborator
AltGr commented April 09, 2013

This patch fixes the issue with color-mode (!) that failed with fatal error when some packages couldn't be built, even if those weren't needed to compile the demanded targets.

However, it still changes the behaviour regarding broken packages, and still breaks bootstrap for 3.12 at the moment. We need to figure out what we want to do with warnings/errors, and possibly fix the Makefile, before merging (I removed the -no-color option so that the problem doesn't stay hidden).

  • Previous behaviour: warnings are printed (at the beginning of compilation) and broken packages are ignored: what is possible is still built, but the warnings may be lost if there is a long build log.
  • Behaviour with this patch (and color mode, to keep the backwards compatibility of the no-color mode: this is of course transitional): we don't issue errors nor warnings about unrelated packages anymore. But it is a fatal error when a package that is in the current targets can't be built.

The latter is better in my opinion, since warnings should'nt be hidden, but causes problems when the target projects aren't specified, because in that case it will fail if any of them is broken. In particular, some projects in typerex use compiler-libs, which is not available on 3.12 : since the Makefile runs ocp-build without specifying the targets, it will try to build all of them, and completely fail instead of building what it can.

A clean, but maybe somewhat heavy solution would be to have an old-school ./configure that detects what projects are available, and to always specify the list of projects to ocp-build. Or to choose a different behaviour for failures, for example when the -k option is specified.

Fabrice Le Fessant
Owner

I propose a third option:

  • it prints warnings if some packages are not complete/available (note that this information has been reduced in the new version, I am a bit worried about that, because it could mean that there is not enough information now to understand and fix the problem, for example now, if a package is missing, there is no way to know which package requires the missing package)
  • it compiles what can be compiled
  • if one package was specified that could not be built because of missing packages (or if no targets were specified), it prints a short error message to tell the user that some packages are missing, and that the complete info is at the beginning, and then exit with code 2

Note that this behavior means that, if a package is missing, but it is not required for specified targets, ocp-build will not fail, i.e. it will exit with code 0.

About ./configure: Unix-only thing, does not exist on Windows, reason why ocp-build does the job itself.

About the bootstrap: it should not fail under 3.12.1, so we must keep -no-color until the old behavior is recovered (an error in the build process is BAD, it cannot be used to remember bugs, the bug-tracker exists for that !)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 1 unique commit by 1 author.

Apr 09, 2013
Louis Gesbert Fixed failing when packages unrelated to what is asked for are missin…
…g/broken
c11d7fd
This page is out of date. Refresh to see the latest.
2  Makefile
@@ -118,7 +118,7 @@ bootstrap-ready: \
118 118
    _obuild/ocp-build/ocp-build.byte
119 119
 
120 120
 old-ocp-build:
121  
-	OCAML_VERSION=ocaml-3.12.1 ocp-build -no-color -arch 3.12.1 ocp-build
  121
+	OCAML_VERSION=ocaml-3.12.1 ocp-build -arch 3.12.1 ocp-build
122 122
 
123 123
 bootstrap: old-ocp-build
124 124
 	rm -rf Saved
67  tools/ocp-build/buildMain.ml
@@ -20,7 +20,7 @@
20 20
 *)
21 21
 
22 22
 (* TODO
23  
-   We could force packages with missing dependencies to still be compiaboutled,
  23
+   We could force packages with missing dependencies to still be compiled,
24 24
    since it is still possible that these missing dependencies arbue not used
25 25
    in a particular compilation scheme.
26 26
 *)
@@ -128,16 +128,20 @@ let do_install install_where install_what projects =
128 128
   in
129 129
   if already_installed <> [] then
130 130
     if !BuildArgs.auto_uninstall then begin
131  
-      Printf.printf "Packages %s are already installed, removing first...\n"
132  
-        (String.concat ", " (List.map bold already_installed));
  131
+      Printf.printf "Package%s %s %s already installed, removing first...\n"
  132
+        (match already_installed with _::_::_ -> "s" | _ -> "")
  133
+        (String.concat ", " (List.map bold already_installed))
  134
+        (match already_installed with _::_::_ -> "are" | _ -> "is");
133 135
       let uninstall_state = BuildOCamlInstall.uninstall_init install_where in
134 136
       List.iter
135 137
         (BuildOCamlInstall.uninstall_by_name uninstall_state)
136 138
         already_installed;
137 139
       BuildOCamlInstall.uninstall_finish uninstall_state
138 140
     end else begin
139  
-      Printf.eprintf "Error: Packages %s are already installed."
140  
-        (String.concat ", " (List.map bold already_installed));
  141
+      Printf.eprintf "Error: Package%s %s %s already installed."
  142
+        (match already_installed with _::_::_ -> "s" | _ -> "")
  143
+        (String.concat ", " (List.map bold already_installed))
  144
+        (match already_installed with _::_::_ -> "are" | _ -> "is");
141 145
       exit 2
142 146
     end;
143 147
 
@@ -302,7 +306,7 @@ let do_print_project_info pj =
302 306
 
303 307
   end
304 308
 
305  
-let do_print_fancy_project_info pj =
  309
+let do_print_fancy_project_info pj targets =
306 310
   let cantbuild = [] in
307 311
   let missing =
308 312
     List.filter
@@ -310,6 +314,42 @@ let do_print_fancy_project_info pj =
310 314
         List.exists (fun pk -> pk.package_source_kind <> "meta") pkgs)
311 315
       pj.project_missing
312 316
   in
  317
+  let incomplete_targets =
  318
+    let incomplete = Array.to_list pj.project_incomplete in
  319
+    if targets = []
  320
+    then
  321
+      List.map (fun pk -> pk.package_name)
  322
+        (List.filter (fun pk -> pk.package_source_kind <> "meta") incomplete)
  323
+    else
  324
+      List.filter
  325
+        (fun target ->
  326
+          List.exists (fun pk -> pk.package_name = target) incomplete)
  327
+        targets
  328
+  in
  329
+  let missing =
  330
+    (* filter out missing packages that the targets don't (transitively)
  331
+       depend on *)
  332
+    let rec aux needed = function
  333
+      | [] -> needed
  334
+      | (name,provides)::r ->
  335
+          if StringSet.mem name needed
  336
+          then aux needed r
  337
+          else if
  338
+            List.exists
  339
+              (fun pk -> StringSet.mem pk.package_name needed)
  340
+              provides
  341
+          then
  342
+            aux (StringSet.add name needed) missing
  343
+          else
  344
+            aux needed r
  345
+    in
  346
+    let needed =
  347
+      List.fold_left (fun n t -> StringSet.add t n)
  348
+        StringSet.empty incomplete_targets
  349
+    in
  350
+    let needed = aux needed missing in
  351
+    List.filter (fun (name,_) -> StringSet.mem name needed) missing
  352
+  in
313 353
   let missing_roots =
314 354
     (* remove all missing pkgs that depend on another to get the missing roots *)
315 355
     List.filter
@@ -357,7 +397,7 @@ let do_print_fancy_project_info pj =
357 397
     end
358 398
   in
359 399
   let cantbuild =
360  
-    if pj.project_incomplete = [||] then cantbuild
  400
+    if incomplete_targets = [] then cantbuild
361 401
     else begin
362 402
       let additional =
363 403
         List.filter
@@ -367,7 +407,8 @@ let do_print_fancy_project_info pj =
367 407
       in
368 408
       if additional <> [] then
369 409
         Printf.eprintf
370  
-          "Additional packages %s can't be built.\n"
  410
+          "Package%s %s can't be built.\n"
  411
+          (match additional with _::_::_ -> "s" | _ -> "")
371 412
           (String.concat ", "
372 413
              (List.map (fun pk -> Printf.sprintf "\027[1m%s\027[m" pk.package_name)
373 414
                 additional));
@@ -515,7 +556,11 @@ let do_compile b cin ncores projects =
515 556
     let t1 = Unix.gettimeofday () in
516 557
 
517 558
     let nerrors = List.length errors in
518  
-    Printf.eprintf
  559
+    if !BuildEngine.stats_command_executed = 0 then
  560
+      Printf.eprintf "%sEverything up-to-date.%s\n%!"
  561
+      (if !BuildArgs.color then "\027[32m" else "")
  562
+      (if !BuildArgs.color then "\027[m" else "")
  563
+    else Printf.eprintf
519 564
       "%s in %.2fs. %d jobs (parallelism %.1fx), %d files generated.\n%!"
520 565
       (if errors = [] then
521 566
          if !BuildArgs.color then "\027[32mBuild Successful\027[m"
@@ -734,7 +779,7 @@ let build targets =
734 779
 
735 780
     if !configure_arg then save_project := true;
736 781
 
737  
-    if !save_project then begin
  782
+    if !save_project && not !BuildArgs.color then begin
738 783
       Printf.fprintf stderr "Updating ocp-build.root\n%!";
739 784
       BuildOptions.must_save_project ()
740 785
     end;
@@ -771,7 +816,7 @@ let build targets =
771 816
     end;
772 817
 
773 818
     if verbose 1 && !BuildArgs.color then
774  
-      do_print_fancy_project_info pj
  819
+      do_print_fancy_project_info pj targets
775 820
     else
776 821
       do_print_project_info pj;
777 822
 
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.