Skip to content

Commit a9ad523

Browse files
kit-ty-katekit-ty-kate
authored
Merge pull request #6696 from rjbou/better-errors
Pretty printing of some error for opam source & build actions
2 parents 2a21c8f + da06f1d commit a9ad523

File tree

5 files changed

+85
-30
lines changed

5 files changed

+85
-30
lines changed

‎master_changes.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ users)
2626
## Actions
2727

2828
## Install
29+
* More fine grained error message in case of bad hash or missing extra-files error (and remove raw fatal error) [#6696 @rjbou]
2930

3031
## Build (package)
3132

@@ -55,6 +56,8 @@ users)
5556
## Exec
5657

5758
## Source
59+
* Better error message, especially in case of `Failure` [#6696 @rjbou]
60+
* Raise a warning instead of an error when an item of `extra-files` is missing [#6696 @rjbou]
5861

5962
## Lint
6063

‎src/client/opamAction.ml‎

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -495,17 +495,37 @@ let prepare_package_source st nv dir =
495495
else None)
496496
xs
497497
in
498-
let bad_hash =
499-
List.filter_map (fun (src, base, hash) ->
500-
if OpamHash.check_file (OpamFilename.to_string src) hash then
501-
(OpamFilename.copy ~src ~dst:(OpamFilename.create dir base); None)
498+
let bad_hash, missing =
499+
List.fold_left (fun (bad_hash, missing) (src, base, hash) ->
500+
if OpamFilename.exists src then
501+
let file = OpamFilename.to_string src in
502+
if OpamHash.check_file file hash then
503+
(OpamFilename.copy ~src ~dst:(OpamFilename.create dir base);
504+
bad_hash, missing)
505+
else
506+
(src::bad_hash), missing
502507
else
503-
Some src) extra_files
508+
bad_hash, (src::missing))
509+
([], []) extra_files
510+
in
511+
let bad_hash_msg bad_hash =
512+
Printf.sprintf "%s: Bad hash for extra-file\n%s"
513+
(OpamPackage.to_string nv)
514+
(OpamStd.Format.itemize OpamFilename.to_string bad_hash)
515+
in
516+
let missing_msg missing =
517+
Printf.sprintf "%s: Missing extra-file for\n%s"
518+
(OpamPackage.to_string nv)
519+
(OpamStd.Format.itemize OpamFilename.to_string missing)
504520
in
505-
if bad_hash = [] then None else
506-
Some (Failure
507-
(Printf.sprintf "Bad hash for %s"
508-
(OpamStd.Format.itemize OpamFilename.to_string bad_hash)));
521+
match bad_hash, missing with
522+
| [], [] -> None
523+
| _::_ as bad_hash, [] ->
524+
Some (Failure (bad_hash_msg bad_hash))
525+
| [], (_::_ as missing) ->
526+
Some (Failure (missing_msg missing))
527+
| bad_hash, missing ->
528+
Some (Failure (bad_hash_msg bad_hash ^ "\n" ^ missing_msg missing))
509529
in
510530
OpamFilename.mkdir dir;
511531
get_extra_sources_job @@+ function Some _ as err -> Done err | None ->

‎src/client/opamCommands.ml‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3820,8 +3820,11 @@ let source cli =
38203820
OpamConsole.formatted_msg "Successfully extracted to %s\n"
38213821
(Dir.to_string dir)
38223822
| Some e ->
3823-
OpamConsole.warning "Some errors extracting to %s: %s\n"
3824-
(Dir.to_string dir) (Printexc.to_string e)
3823+
OpamConsole.warning "Some errors extracting to %s:\n %s\n"
3824+
(Dir.to_string dir)
3825+
(match e with
3826+
| Failure msg -> msg
3827+
| _ -> Printexc.to_string e)
38253828
in
38263829
OpamProcess.Job.run job;
38273830
if OpamPinned.find_opam_file_in_source nv.name

‎tests/reftests/extrafile.test‎

Lines changed: 46 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,8 @@ The following actions will be performed:
260260

261261
<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
262262

263-
Bad hash for - ${BASEDIR}/OPAM/repo/default/packages/bad-md5/bad-md5.1/files/p.patch
263+
bad-md5.1: Bad hash for extra-file
264+
- ${BASEDIR}/OPAM/repo/default/packages/bad-md5/bad-md5.1/files/p.patch
264265

265266

266267

@@ -277,7 +278,8 @@ The following actions will be performed:
277278

278279
<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
279280

280-
Bad hash for - ${BASEDIR}/OPAM/repo/default/packages/bad-md5/bad-md5.1/files/p.patch
281+
bad-md5.1: Bad hash for extra-file
282+
- ${BASEDIR}/OPAM/repo/default/packages/bad-md5/bad-md5.1/files/p.patch
281283

282284

283285

@@ -288,13 +290,19 @@ Bad hash for - ${BASEDIR}/OPAM/repo/default/packages/bad-md5/bad-md5.1/files/p
288290
- No changes have been performed
289291
# Return code 31 #
290292
### opam source bad-md5 | '.n"' -> '"'
291-
[WARNING] Some errors extracting to ${BASEDIR}/bad-md5.1: Failure("Bad hash for - ${BASEDIR}/OPAM/repo/default/packages/bad-md5/bad-md5.1/files/p.patch")
293+
[WARNING] Some errors extracting to ${BASEDIR}/bad-md5.1:
294+
bad-md5.1: Bad hash for extra-file
295+
- ${BASEDIR}/OPAM/repo/default/packages/bad-md5/bad-md5.1/files/p.patch
296+
292297

293298
### test -f bad-md5.1/p.patch
294299
# Return code 1 #
295300
### rm -r bad-md5.1
296301
### opam source bad-md5 --require-checksums | '.n"' -> '"'
297-
[WARNING] Some errors extracting to ${BASEDIR}/bad-md5.1: Failure("Bad hash for - ${BASEDIR}/OPAM/repo/default/packages/bad-md5/bad-md5.1/files/p.patch")
302+
[WARNING] Some errors extracting to ${BASEDIR}/bad-md5.1:
303+
bad-md5.1: Bad hash for extra-file
304+
- ${BASEDIR}/OPAM/repo/default/packages/bad-md5/bad-md5.1/files/p.patch
305+
298306

299307
### test -f bad-md5.1/p.patch
300308
# Return code 1 #
@@ -316,7 +324,8 @@ The following actions will be performed:
316324

317325
<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
318326

319-
Bad hash for - ${BASEDIR}/OPAM/repo/default/packages/good-md5-bad-sha256/good-md5-bad-sha256.1/files/p.patch
327+
good-md5-bad-sha256.1: Bad hash for extra-file
328+
- ${BASEDIR}/OPAM/repo/default/packages/good-md5-bad-sha256/good-md5-bad-sha256.1/files/p.patch
320329

321330

322331

@@ -333,7 +342,8 @@ The following actions will be performed:
333342

334343
<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
335344

336-
Bad hash for - ${BASEDIR}/OPAM/repo/default/packages/good-md5-bad-sha256/good-md5-bad-sha256.1/files/p.patch
345+
good-md5-bad-sha256.1: Bad hash for extra-file
346+
- ${BASEDIR}/OPAM/repo/default/packages/good-md5-bad-sha256/good-md5-bad-sha256.1/files/p.patch
337347

338348

339349

@@ -344,12 +354,18 @@ Bad hash for - ${BASEDIR}/OPAM/repo/default/packages/good-md5-bad-sha256/good-
344354
- No changes have been performed
345355
# Return code 31 #
346356
### opam source good-md5-bad-sha256 | '.n"' -> '"'
347-
[WARNING] Some errors extracting to ${BASEDIR}/good-md5-bad-sha256.1: Failure("Bad hash for - ${BASEDIR}/OPAM/repo/default/packages/good-md5-bad-sha256/good-md5-bad-sha256.1/files/p.patch")
357+
[WARNING] Some errors extracting to ${BASEDIR}/good-md5-bad-sha256.1:
358+
good-md5-bad-sha256.1: Bad hash for extra-file
359+
- ${BASEDIR}/OPAM/repo/default/packages/good-md5-bad-sha256/good-md5-bad-sha256.1/files/p.patch
360+
348361

349362
### test -f good-md5-bad-sha256.1/p.patch
350363
### rm -r good-md5-bad-sha256.1
351364
### opam source good-md5-bad-sha256 --require-checksums | '.n"' -> '"'
352-
[WARNING] Some errors extracting to ${BASEDIR}/good-md5-bad-sha256.1: Failure("Bad hash for - ${BASEDIR}/OPAM/repo/default/packages/good-md5-bad-sha256/good-md5-bad-sha256.1/files/p.patch")
365+
[WARNING] Some errors extracting to ${BASEDIR}/good-md5-bad-sha256.1:
366+
good-md5-bad-sha256.1: Bad hash for extra-file
367+
- ${BASEDIR}/OPAM/repo/default/packages/good-md5-bad-sha256/good-md5-bad-sha256.1/files/p.patch
368+
353369

354370
### test -f good-md5-bad-sha256.1/p.patch
355371
### opam clean --download-cache
@@ -494,7 +510,9 @@ The following actions will be performed:
494510

495511
<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
496512

497-
Sys_error("${BASEDIR}/OPAM/repo/default/packages/not-present/not-present.1/files/p.patch: No such file or directory")
513+
not-present.1: Missing extra-file for
514+
- ${BASEDIR}/OPAM/repo/default/packages/not-present/not-present.1/files/p.patch
515+
498516

499517

500518
<><> Error report <><><><><><><><><><><><><><><><><><><><><><><><><><><><><><><>
@@ -510,7 +528,9 @@ The following actions will be performed:
510528

511529
<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
512530

513-
Sys_error("${BASEDIR}/OPAM/repo/default/packages/not-present/not-present.1/files/p.patch: No such file or directory")
531+
not-present.1: Missing extra-file for
532+
- ${BASEDIR}/OPAM/repo/default/packages/not-present/not-present.1/files/p.patch
533+
514534

515535

516536
<><> Error report <><><><><><><><><><><><><><><><><><><><><><><><><><><><><><><>
@@ -520,9 +540,11 @@ Sys_error("${BASEDIR}/OPAM/repo/default/packages/not-present/not-present.1/files
520540
- No changes have been performed
521541
# Return code 31 #
522542
### opam source not-present
523-
Fatal error:
524-
Sys_error("${BASEDIR}/OPAM/repo/default/packages/not-present/not-present.1/files/p.patch: No such file or directory")
525-
# Return code 99 #
543+
[WARNING] Some errors extracting to ${BASEDIR}/not-present.1:
544+
not-present.1: Missing extra-file for
545+
- ${BASEDIR}/OPAM/repo/default/packages/not-present/not-present.1/files/p.patch
546+
547+
526548
### test -f not-present.1/p.patch
527549
# Return code 1 #
528550
### opam clean --download-cache
@@ -546,7 +568,9 @@ The following actions will be performed:
546568

547569
<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
548570

549-
Sys_error("${BASEDIR}/OPAM/repo/default/packages/escape-absolute/escape-absolute.1/files/etc/passwdd: No such file or directory")
571+
escape-absolute.1: Missing extra-file for
572+
- ${BASEDIR}/OPAM/repo/default/packages/escape-absolute/escape-absolute.1/files/etc/passwdd
573+
550574

551575

552576
<><> Error report <><><><><><><><><><><><><><><><><><><><><><><><><><><><><><><>
@@ -562,7 +586,9 @@ The following actions will be performed:
562586

563587
<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
564588

565-
Sys_error("${BASEDIR}/OPAM/repo/default/packages/escape-absolute/escape-absolute.1/files/etc/passwdd: No such file or directory")
589+
escape-absolute.1: Missing extra-file for
590+
- ${BASEDIR}/OPAM/repo/default/packages/escape-absolute/escape-absolute.1/files/etc/passwdd
591+
566592

567593

568594
<><> Error report <><><><><><><><><><><><><><><><><><><><><><><><><><><><><><><>
@@ -572,9 +598,11 @@ Sys_error("${BASEDIR}/OPAM/repo/default/packages/escape-absolute/escape-absolute
572598
- No changes have been performed
573599
# Return code 31 #
574600
### opam source escape-absolute
575-
Fatal error:
576-
Sys_error("${BASEDIR}/OPAM/repo/default/packages/escape-absolute/escape-absolute.1/files/etc/passwdd: No such file or directory")
577-
# Return code 99 #
601+
[WARNING] Some errors extracting to ${BASEDIR}/escape-absolute.1:
602+
escape-absolute.1: Missing extra-file for
603+
- ${BASEDIR}/OPAM/repo/default/packages/escape-absolute/escape-absolute.1/files/etc/passwdd
604+
605+
578606
### test -f escape-absolute.1/p.patch
579607
# Return code 1 #
580608
### opam clean --download-cache

‎tests/reftests/extrasource.test‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -559,7 +559,8 @@ Successfully extracted to ${BASEDIR}/no-checksum.1
559559
### test -f no-checksum.1/i-am-a-patch
560560
### rm -r no-checksum.1
561561
### opam source no-checksum --require-checksums
562-
[WARNING] Some errors extracting to ${BASEDIR}/no-checksum.1: Failure("no-checksum.1/i-am-a-patch: Missing checksum, and `--require-checksums` was set.")
562+
[WARNING] Some errors extracting to ${BASEDIR}/no-checksum.1:
563+
no-checksum.1/i-am-a-patch: Missing checksum, and `--require-checksums` was set.
563564

564565
### test -f no-checksum.1/i-am-a-patch
565566
# Return code 1 #

0 commit comments

Comments
 (0)