diff options
Diffstat (limited to 'pkgtools/pkglint/files/substcontext_test.go')
| -rw-r--r-- | pkgtools/pkglint/files/substcontext_test.go | 1312 |
1 files changed, 755 insertions, 557 deletions
diff --git a/pkgtools/pkglint/files/substcontext_test.go b/pkgtools/pkglint/files/substcontext_test.go index f8bd4b67e99..8cae3f3c451 100644 --- a/pkgtools/pkglint/files/substcontext_test.go +++ b/pkgtools/pkglint/files/substcontext_test.go @@ -5,6 +5,7 @@ import "gopkg.in/check.v1" func (t *Tester) NewSubstAutofixTest(lines ...string) func(bool) { return func(autofix bool) { mklines := t.NewMkLines("filename.mk", lines...) + mklines.collectRationale() ctx := NewSubstContext() mklines.ForEach(ctx.Process) @@ -18,111 +19,13 @@ func (t *Tester) RunSubst(lines ...string) { assert(lines[len(lines)-1] != "") mklines := t.NewMkLines("filename.mk", lines...) + mklines.collectRationale() ctx := NewSubstContext() mklines.ForEach(ctx.Process) ctx.Finish(mklines.EOFLine()) } -func (s *Suite) Test_SubstContext__OPSYSVARS(c *check.C) { - t := s.Init(c) - - ctx := NewSubstContext() - - // SUBST_CLASSES is added to OPSYSVARS in mk/bsd.pkg.mk. - ctx.varassign(t.NewMkLine("filename.mk", 11, "SUBST_CLASSES.SunOS+=prefix")) - ctx.varassign(t.NewMkLine("filename.mk", 12, "SUBST_CLASSES.NetBSD+=prefix")) - ctx.varassign(t.NewMkLine("filename.mk", 13, "SUBST_FILES.prefix=Makefile")) - ctx.varassign(t.NewMkLine("filename.mk", 14, "SUBST_SED.prefix=s,@PREFIX@,${PREFIX},g")) - ctx.varassign(t.NewMkLine("filename.mk", 15, "SUBST_STAGE.prefix=post-configure")) - - t.CheckEquals(ctx.isComplete(), true) - - ctx.Finish(t.NewMkLine("filename.mk", 15, "")) - - t.CheckOutputLines( - "NOTE: filename.mk:14: The substitution command \"s,@PREFIX@,${PREFIX},g\" " + - "can be replaced with \"SUBST_VARS.prefix= PREFIX\".") -} - -func (s *Suite) Test_SubstContext__no_class(c *check.C) { - t := s.Init(c) - - t.RunSubst( - "UNRELATED=anything", - "SUBST_FILES.repl+=Makefile.in", - "SUBST_SED.repl+=-e s,from,to,g") - - t.CheckOutputLines( - "WARN: filename.mk:2: Before defining SUBST_FILES.repl, " + - "the SUBST class should be declared using \"SUBST_CLASSES+= repl\".") -} - -func (s *Suite) Test_SubstContext__multiple_classes_in_one_line(c *check.C) { - t := s.Init(c) - - t.RunSubst( - "SUBST_CLASSES+= one two", - "SUBST_STAGE.one= post-configure", - "SUBST_FILES.one= one.txt", - "SUBST_SED.one= s,one,1,g", - "SUBST_STAGE.two= post-configure", - "SUBST_FILES.two= two.txt") - - t.CheckOutputLines( - "NOTE: filename.mk:1: Please add only one class at a time to SUBST_CLASSES.", - "WARN: filename.mk:EOF: Incomplete SUBST block: SUBST_SED.two, SUBST_VARS.two or SUBST_FILTER_CMD.two missing.") -} - -func (s *Suite) Test_SubstContext__multiple_classes_in_one_line_multiple_blocks(c *check.C) { - t := s.Init(c) - - t.RunSubst( - "SUBST_CLASSES+= one two", - "SUBST_STAGE.one= post-configure", - "SUBST_FILES.one= one.txt", - "SUBST_SED.one= s,one,1,g", - "", - "SUBST_STAGE.two= post-configure", - "SUBST_FILES.two= two.txt", - "", - "SUBST_STAGE.three= post-configure", - "", - "SUBST_VARS.four= PREFIX", - "", - "SUBST_VARS.three= PREFIX") - - t.CheckOutputLines( - "NOTE: filename.mk:1: Please add only one class at a time to SUBST_CLASSES.", - "WARN: filename.mk:8: Incomplete SUBST block: "+ - "SUBST_SED.two, SUBST_VARS.two or SUBST_FILTER_CMD.two missing.", - "WARN: filename.mk:9: Before defining SUBST_STAGE.three, "+ - "the SUBST class should be declared using \"SUBST_CLASSES+= three\".", - "WARN: filename.mk:11: Before defining SUBST_VARS.four, "+ - "the SUBST class should be declared using \"SUBST_CLASSES+= four\".") -} - -func (s *Suite) Test_SubstContext__multiple_classes_in_one_block(c *check.C) { - t := s.Init(c) - - t.RunSubst( - "SUBST_CLASSES+= one", - "SUBST_STAGE.one= post-configure", - "SUBST_STAGE.one= post-configure", - "SUBST_FILES.one= one.txt", - "SUBST_CLASSES+= two", // The block "one" is not finished yet. - "SUBST_SED.one= s,one,1,g", - "SUBST_STAGE.two= post-configure", - "SUBST_FILES.two= two.txt", - "SUBST_SED.two= s,two,2,g") - - t.CheckOutputLines( - "WARN: filename.mk:3: Duplicate definition of \"SUBST_STAGE.one\".", - "WARN: filename.mk:5: Incomplete SUBST block: SUBST_SED.one, SUBST_VARS.one or SUBST_FILTER_CMD.one missing.", - "WARN: filename.mk:5: Subst block \"one\" should be finished before adding the next class to SUBST_CLASSES.", - "WARN: filename.mk:6: Variable \"SUBST_SED.one\" does not match SUBST class \"two\".") -} - // This is a strange example that probably won't occur in practice. // // Continuing a SUBST class in one of the branches and starting @@ -147,26 +50,7 @@ func (s *Suite) Test_SubstContext__partially_continued_class_in_conditional(c *c t.CheckOutputEmpty() } -func (s *Suite) Test_SubstContext__files_missing(c *check.C) { - t := s.Init(c) - - t.RunSubst( - "SUBST_CLASSES+= one", - "SUBST_STAGE.one= pre-configure", - "SUBST_CLASSES+= two", - "SUBST_STAGE.two= pre-configure", - "SUBST_FILES.two= two.txt", - "SUBST_SED.two= s,two,2,g") - - t.CheckOutputLines( - "WARN: filename.mk:3: Incomplete SUBST block: SUBST_FILES.one missing.", - "WARN: filename.mk:3: Incomplete SUBST block: "+ - "SUBST_SED.one, SUBST_VARS.one or SUBST_FILTER_CMD.one missing.", - "WARN: filename.mk:3: Subst block \"one\" should be finished "+ - "before adding the next class to SUBST_CLASSES.") -} - -func (s *Suite) Test_SubstContext__directives(c *check.C) { +func (s *Suite) Test_SubstContext__conditionals(c *check.C) { t := s.Init(c) t.RunSubst( @@ -193,88 +77,75 @@ func (s *Suite) Test_SubstContext__directives(c *check.C) { "to \"SUBST_SED.os\" should use the \"+=\" operator.") } -func (s *Suite) Test_SubstContext__adjacent(c *check.C) { +func (s *Suite) Test_SubstContext_varassign__no_class(c *check.C) { t := s.Init(c) t.RunSubst( - "SUBST_CLASSES+=\t1", - "SUBST_STAGE.1=\tpre-configure", - "SUBST_FILES.1=\tfile1", - "SUBST_SED.1=\t-e s,subst1,repl1,", - "SUBST_CLASSES+=\t2", - "SUBST_SED.1+=\t-e s,subst1b,repl1b,", // Misplaced - "SUBST_STAGE.2=\tpre-configure", - "SUBST_FILES.2=\tfile2", - "SUBST_SED.2=\t-e s,subst2,repl2,") - - t.CheckOutputLines( - "WARN: filename.mk:6: Variable \"SUBST_SED.1\" does not match SUBST class \"2\".") -} - -func (s *Suite) Test_SubstContext__do_patch(c *check.C) { - t := s.Init(c) - - t.RunSubst( - "SUBST_CLASSES+=\tos", - "SUBST_STAGE.os=\tdo-patch", - "SUBST_FILES.os=\tguess-os.h", - "SUBST_SED.os=\t-e s,@OPSYS@,Darwin,") - - // No warning, since there is nothing to fix automatically. - // This case doesn't occur in practice anyway. - t.CheckOutputEmpty() -} - -// Variables mentioned in SUBST_VARS are not considered "foreign" -// in the block and may be mixed with the other SUBST variables. -func (s *Suite) Test_SubstContext__SUBST_VARS_defined_in_block(c *check.C) { - t := s.Init(c) - - t.RunSubst( - "SUBST_CLASSES+=\tos", - "SUBST_STAGE.os=\tpre-configure", - "SUBST_FILES.os=\tguess-os.h", - "SUBST_VARS.os=\tTODAY1", - "TODAY1!=\tdate", - "TODAY2!=\tdate") + "UNRELATED=anything", + "SUBST_FILES.repl+=Makefile.in", + "SUBST_SED.repl+=-e s,from,to,g") t.CheckOutputLines( - "WARN: filename.mk:6: Foreign variable \"TODAY2\" in SUBST block.") + "WARN: filename.mk:2: Before defining SUBST_FILES.repl, " + + "the SUBST class should be declared using \"SUBST_CLASSES+= repl\".") } -// Variables mentioned in SUBST_VARS may appear in the same paragraph, -// or alternatively anywhere else in the file. -func (s *Suite) Test_SubstContext__SUBST_VARS_in_next_paragraph(c *check.C) { +func (s *Suite) Test_SubstContext_varassign__multiple_classes_in_one_line_multiple_blocks(c *check.C) { t := s.Init(c) t.RunSubst( - "SUBST_CLASSES+=\tos", - "SUBST_STAGE.os=\tpre-configure", - "SUBST_FILES.os=\tguess-os.h", - "SUBST_VARS.os=\tTODAY1", + "SUBST_CLASSES+= one two", + "SUBST_STAGE.one= post-configure", + "SUBST_FILES.one= one.txt", + "SUBST_SED.one= s,one,1,g", "", - "TODAY1!=\tdate", - "TODAY2!=\tdate") + "SUBST_STAGE.two= post-configure", + "SUBST_FILES.two= two.txt", + "", + "SUBST_STAGE.three= post-configure", + "", + "SUBST_VARS.four= PREFIX", + "", + "SUBST_VARS.three= PREFIX") - t.CheckOutputEmpty() + t.CheckOutputLines( + "NOTE: filename.mk:1: Please add only one class at a time to SUBST_CLASSES.", + "WARN: filename.mk:9: Variable \"SUBST_STAGE.three\" "+ + "does not match SUBST class \"two\".", + "WARN: filename.mk:9: Incomplete SUBST block: "+ + "SUBST_SED.two, SUBST_VARS.two or SUBST_FILTER_CMD.two missing.", + "WARN: filename.mk:9: Before defining SUBST_STAGE.three, "+ + "the SUBST class should be declared using \"SUBST_CLASSES+= three\".", + "WARN: filename.mk:11: Before defining SUBST_VARS.four, "+ + "the SUBST class should be declared using \"SUBST_CLASSES+= four\".") } -func (s *Suite) Test_SubstContext__multiple_SUBST_VARS(c *check.C) { +func (s *Suite) Test_SubstContext_varassign__multiple_classes_in_one_block(c *check.C) { t := s.Init(c) t.RunSubst( - "SUBST_CLASSES+=\tos", - "SUBST_STAGE.os=\tpre-configure", - "SUBST_FILES.os=\tguess-os.h", - "SUBST_VARS.os=\tPREFIX VARBASE") + "SUBST_CLASSES+= one", + "SUBST_STAGE.one= post-configure", + "SUBST_STAGE.one= post-configure", + "SUBST_FILES.one= one.txt", + "SUBST_CLASSES+= two", // The block "one" is not finished yet. + "SUBST_SED.one= s,one,1,g", + "SUBST_STAGE.two= post-configure", + "SUBST_FILES.two= two.txt", + "SUBST_SED.two= s,two,2,g") - t.CheckOutputEmpty() + t.CheckOutputLines( + "WARN: filename.mk:3: Duplicate definition of \"SUBST_STAGE.one\".", + "WARN: filename.mk:5: Subst block \"one\" should be finished "+ + "before adding the next class to SUBST_CLASSES.", + "WARN: filename.mk:5: Incomplete SUBST block: SUBST_SED.one, SUBST_VARS.one or SUBST_FILTER_CMD.one missing.", + "WARN: filename.mk:6: Late additions to a SUBST variable should use the += operator.") } -// As of May 2019, pkglint does not check the order of the variables in +// As of December 2019, pkglint does not check the order of the variables in // a SUBST block. Enforcing this order, or at least suggesting it, would // make pkgsrc packages more uniform, which is a good idea, but not urgent. -func (s *Suite) Test_SubstContext__unusual_variable_order(c *check.C) { +func (s *Suite) Test_SubstContext_varassign__unusual_order(c *check.C) { t := s.Init(c) t.RunSubst( @@ -287,41 +158,7 @@ func (s *Suite) Test_SubstContext__unusual_variable_order(c *check.C) { t.CheckOutputEmpty() } -func (s *Suite) Test_SubstContext__completely_conditional_then(c *check.C) { - t := s.Init(c) - - t.RunSubst( - ".if ${OPSYS} == Linux", - "SUBST_CLASSES+=\tid", - "SUBST_STAGE.id=\tpre-configure", - "SUBST_SED.id=\t-e sahara", - ".else", - ".endif") - - // The block already ends at the .else, not at the end of the file, - // since that is the scope where the SUBST id is defined. - t.CheckOutputLines( - "WARN: filename.mk:5: Incomplete SUBST block: SUBST_FILES.id missing.") -} - -func (s *Suite) Test_SubstContext__completely_conditional_else(c *check.C) { - t := s.Init(c) - - t.RunSubst( - ".if ${OPSYS} == Linux", - ".else", - "SUBST_CLASSES+=\tid", - "SUBST_STAGE.id=\tpre-configure", - "SUBST_SED.id=\t-e sahara", - ".endif") - - // The block already ends at the .endif, not at the end of the file, - // since that is the scope where the SUBST id is defined. - t.CheckOutputLines( - "WARN: filename.mk:6: Incomplete SUBST block: SUBST_FILES.id missing.") -} - -func (s *Suite) Test_SubstContext__SUBST_CLASSES_in_separate_paragraph(c *check.C) { +func (s *Suite) Test_SubstContext_varassign__blocks_in_separate_paragraphs(c *check.C) { t := s.Init(c) t.RunSubst( @@ -344,7 +181,7 @@ func (s *Suite) Test_SubstContext__SUBST_CLASSES_in_separate_paragraph(c *check. "WARN: filename.mk:EOF: Missing SUBST block for \"4\".") } -func (s *Suite) Test_SubstContext__wrong_class(c *check.C) { +func (s *Suite) Test_SubstContext_varassign__typo_in_id(c *check.C) { t := s.Init(c) t.RunSubst( @@ -358,33 +195,9 @@ func (s *Suite) Test_SubstContext__wrong_class(c *check.C) { t.CheckOutputLines( "NOTE: filename.mk:1: Please add only one class at a time to SUBST_CLASSES.", - "WARN: filename.mk:2: Variable \"SUBST_STAGE.x\" does not match SUBST class \"1\".", - "WARN: filename.mk:3: Variable \"SUBST_FILES.x\" does not match SUBST class \"1\".", - "WARN: filename.mk:4: Variable \"SUBST_VARS.x\" does not match SUBST class \"1\".", - // XXX: This line could change to 2, since that is already in the queue. - "WARN: filename.mk:5: Variable \"SUBST_STAGE.2\" does not match SUBST class \"1\".", - "WARN: filename.mk:6: Variable \"SUBST_FILES.2\" does not match SUBST class \"1\".", - "WARN: filename.mk:7: Variable \"SUBST_VARS.2\" does not match SUBST class \"1\".", - "WARN: filename.mk:EOF: Missing SUBST block for \"1\".", - "WARN: filename.mk:EOF: Missing SUBST block for \"2\".") -} - -func (s *Suite) Test_SubstContext_varassign__late_addition(c *check.C) { - t := s.Init(c) - - t.RunSubst( - "SUBST_CLASSES+=\tid", - "SUBST_STAGE.id=\tpost-configure", - "SUBST_FILES.id=\tfiles", - "SUBST_VARS.id=\tPREFIX", - "", - ".if ${OPSYS} == NetBSD", - "SUBST_VARS.id=\tOPSYS", - ".endif") - - t.CheckOutputLines( - "WARN: filename.mk:7: Late additions to a SUBST variable " + - "should use the += operator.") + "WARN: filename.mk:2: Before defining SUBST_STAGE.x, "+ + "the SUBST class should be declared using \"SUBST_CLASSES+= x\".", + "WARN: filename.mk:EOF: Missing SUBST block for \"1\".") } func (s *Suite) Test_SubstContext_varassign__late_addition_to_unknown_class(c *check.C) { @@ -403,6 +216,93 @@ func (s *Suite) Test_SubstContext_varassign__late_addition_to_unknown_class(c *c "the SUBST class should be declared using \"SUBST_CLASSES+= id\".") } +func (s *Suite) Test_SubstContext_varassign__rationale(c *check.C) { + t := s.Init(c) + + t.RunSubst( + "# Adjust setup.py", + "SUBST_CLASSES+= setup", + "SUBST_STAGE.setup= post-configure", + "SUBST_FILES.setup= setup.py", + "SUBST_VARS.setup= VAR") + + // The rationale in line 1 is supposed to suppress warnings, + // not add new ones. + t.CheckOutputEmpty() +} + +func (s *Suite) Test_SubstContext_varassign__interleaved(c *check.C) { + t := s.Init(c) + + t.RunSubst( + "SUBST_CLASSES+= 1 2 3", + "SUBST_STAGE.1= post-configure", + "SUBST_STAGE.2= post-configure", + "SUBST_STAGE.3= post-configure", + "SUBST_FILES.1= setup.py", + "SUBST_FILES.2= setup.py", + "SUBST_FILES.3= setup.py", + "SUBST_VARS.1= VAR", + "SUBST_VARS.2= VAR", + "SUBST_VARS.3= VAR") + + // The above does not follow the common pattern of defining + // each block on its own. + // It technically works but is not easy to read for humans. + t.CheckOutputLines( + "NOTE: filename.mk:1: " + + "Please add only one class at a time to SUBST_CLASSES.") +} + +func (s *Suite) Test_SubstContext_varassignClasses__OPSYSVARS(c *check.C) { + t := s.Init(c) + + ctx := NewSubstContext() + + // SUBST_CLASSES is added to OPSYSVARS in mk/bsd.pkg.mk. + ctx.varassign(t.NewMkLine("filename.mk", 11, "SUBST_CLASSES.SunOS+=prefix")) + ctx.varassign(t.NewMkLine("filename.mk", 12, "SUBST_CLASSES.NetBSD+=prefix")) + ctx.varassign(t.NewMkLine("filename.mk", 13, "SUBST_FILES.prefix=Makefile")) + ctx.varassign(t.NewMkLine("filename.mk", 14, "SUBST_SED.prefix=s,@PREFIX@,${PREFIX},g")) + ctx.varassign(t.NewMkLine("filename.mk", 15, "SUBST_STAGE.prefix=post-configure")) + + t.CheckEquals(ctx.block().isComplete(), true) + + ctx.Finish(t.NewMkLine("filename.mk", 15, "")) + + t.CheckOutputLines( + "NOTE: filename.mk:14: The substitution command \"s,@PREFIX@,${PREFIX},g\" " + + "can be replaced with \"SUBST_VARS.prefix= PREFIX\".") +} + +func (s *Suite) Test_SubstContext_varassignClasses__duplicate_id(c *check.C) { + t := s.Init(c) + + t.RunSubst( + "SUBST_CLASSES+= id", + "SUBST_CLASSES+= id") + + t.CheckOutputLines( + "ERROR: filename.mk:2: Duplicate SUBST class \"id\".", + "WARN: filename.mk:EOF: Missing SUBST block for \"id\".") +} + +func (s *Suite) Test_SubstContext_varassignClasses__multiple_classes_in_one_line(c *check.C) { + t := s.Init(c) + + t.RunSubst( + "SUBST_CLASSES+= one two", + "SUBST_STAGE.one= post-configure", + "SUBST_FILES.one= one.txt", + "SUBST_SED.one= s,one,1,g", + "SUBST_STAGE.two= post-configure", + "SUBST_FILES.two= two.txt") + + t.CheckOutputLines( + "NOTE: filename.mk:1: Please add only one class at a time to SUBST_CLASSES.", + "WARN: filename.mk:EOF: Incomplete SUBST block: SUBST_SED.two, SUBST_VARS.two or SUBST_FILTER_CMD.two missing.") +} + func (s *Suite) Test_SubstContext_varassignClasses__none(c *check.C) { t := s.Init(c) @@ -428,6 +328,44 @@ func (s *Suite) Test_SubstContext_varassignClasses__indirect(c *check.C) { "WARN: filename.mk:2: VAR is used but not defined.") } +func (s *Suite) Test_SubstContext_varassignOutsideBlock__assign(c *check.C) { + t := s.Init(c) + + t.RunSubst( + "SUBST_CLASSES+=\t1", + "SUBST_STAGE.1=\tpre-configure", + "SUBST_FILES.1=\tfile1", + "SUBST_SED.1=\t-e s,subst1,repl1,", + "SUBST_CLASSES+=\t2", + "SUBST_SED.1=\t-e s,subst1b,repl1b,", // Misplaced + "SUBST_STAGE.2=\tpre-configure", + "SUBST_FILES.2=\tfile2", + "SUBST_SED.2=\t-e s,subst2,repl2,") + + t.CheckOutputLines( + "WARN: filename.mk:6: Late additions to a SUBST variable " + + "should use the += operator.") +} + +func (s *Suite) Test_SubstContext_varassignOutsideBlock__append(c *check.C) { + t := s.Init(c) + + t.RunSubst( + "SUBST_CLASSES+=\t1", + "SUBST_STAGE.1=\tpre-configure", + "SUBST_FILES.1=\tfile1", + "SUBST_SED.1=\t-e s,subst1,repl1,", + "SUBST_CLASSES+=\t2", + "SUBST_SED.1+=\t-e s,subst1b,repl1b,", // Misplaced + "SUBST_STAGE.2=\tpre-configure", + "SUBST_FILES.2=\tfile2", + "SUBST_SED.2=\t-e s,subst2,repl2,") + + // The SUBST_SED.1 line is misplaced. It uses the += operator, + // which makes it unclear whether this is a typo or not. + t.CheckOutputEmpty() +} + // The rationale for the stray SUBST variables has to be specific. // // For example, in the following snippet from mail/dkim-milter/options.mk @@ -472,7 +410,420 @@ func (s *Suite) Test_SubstContext_varassignOutsideBlock__rationale(c *check.C) { "the SUBST class should be declared using \"SUBST_CLASSES+= libs\".") } -func (s *Suite) Test_SubstContext_varassignStage__pre_patch(c *check.C) { +// Unbalanced conditionals must not lead to a panic. +func (s *Suite) Test_SubstContext_directive__before_SUBST_CLASSES(c *check.C) { + t := s.Init(c) + + t.RunSubst( + ".if 0", + ".endif", + "SUBST_CLASSES+=\tos", + ".elif 0") + + t.CheckOutputLines( + "WARN: filename.mk:4: Missing SUBST block for \"os\".") +} + +func (s *Suite) Test_SubstContext_directive__conditional_blocks_complete(c *check.C) { + t := s.Init(c) + + t.RunSubst( + ".if ${OPSYS} == NetBSD", + "SUBST_CLASSES+= nb", + "SUBST_STAGE.nb= post-configure", + "SUBST_FILES.nb= guess-netbsd.h", + "SUBST_VARS.nb= HAVE_NETBSD", + ".else", + "SUBST_CLASSES+= os", + "SUBST_STAGE.os= post-configure", + "SUBST_FILES.os= guess-netbsd.h", + "SUBST_VARS.os= HAVE_OTHER", + ".endif") + + t.CheckOutputEmpty() +} + +func (s *Suite) Test_SubstContext_directive__conditional_blocks_incomplete(c *check.C) { + t := s.Init(c) + + t.RunSubst( + ".if ${OPSYS} == NetBSD", + "SUBST_CLASSES+= nb", + "SUBST_STAGE.nb= post-configure", + "SUBST_VARS.nb= HAVE_NETBSD", + ".else", + "SUBST_CLASSES+= os", + "SUBST_STAGE.os= post-configure", + "SUBST_FILES.os= guess-netbsd.h", + ".endif") + + t.CheckOutputLines( + "WARN: filename.mk:5: Incomplete SUBST block: SUBST_FILES.nb missing.", + "WARN: filename.mk:6: Subst block \"nb\" should be finished "+ + "before adding the next class to SUBST_CLASSES.", + "WARN: filename.mk:9: Incomplete SUBST block: "+ + "SUBST_SED.os, SUBST_VARS.os or SUBST_FILTER_CMD.os missing.") +} + +func (s *Suite) Test_SubstContext_directive__conditional_complete(c *check.C) { + t := s.Init(c) + + t.RunSubst( + "SUBST_CLASSES+= id", + ".if ${OPSYS} == NetBSD", + "SUBST_STAGE.id=\t\tpost-configure", + "SUBST_MESSAGE.id=\tpost-configure", + "SUBST_FILES.id=\t\tguess-netbsd.h", + "SUBST_SED.id=\t\t-e s,from,to,", + "SUBST_VARS.id=\t\tHAVE_OTHER", + "SUBST_FILTER_CMD.id=\tHAVE_OTHER", + ".else", + "SUBST_STAGE.id=\t\tpost-configure", + "SUBST_MESSAGE.id=\tpost-configure", + "SUBST_FILES.id=\t\tguess-netbsd.h", + "SUBST_SED.id=\t\t-e s,from,to,", + "SUBST_VARS.id=\t\tHAVE_OTHER", + "SUBST_FILTER_CMD.id=\tHAVE_OTHER", + ".endif") + + t.CheckOutputLines( + "WARN: filename.mk:3: SUBST_STAGE.id should not be defined conditionally.", + "WARN: filename.mk:4: SUBST_MESSAGE.id should not be defined conditionally.", + "WARN: filename.mk:10: SUBST_STAGE.id should not be defined conditionally.", + "WARN: filename.mk:11: SUBST_MESSAGE.id should not be defined conditionally.") +} + +func (s *Suite) Test_SubstContext_directive__conditionally_overwritten_filter(c *check.C) { + t := s.Init(c) + + t.RunSubst( + "SUBST_CLASSES+= id", + "SUBST_STAGE.id=\t\tpost-configure", + "SUBST_MESSAGE.id=\tpost-configure", + "SUBST_FILES.id=\t\tguess-netbsd.h", + "SUBST_FILTER_CMD.id=\tHAVE_OTHER", + ".if ${OPSYS} == NetBSD", + "SUBST_FILTER_CMD.id=\tHAVE_OTHER", + ".endif") + + t.CheckOutputLines( + "WARN: filename.mk:7: Duplicate definition of \"SUBST_FILTER_CMD.id\".") +} + +// Hopefully nobody will ever trigger this case in real pkgsrc. +// It's plain confusing to a casual reader to nest a complete +// SUBST block into another SUBST block. +func (s *Suite) Test_SubstContext_directive__conditionally_nested_block(c *check.C) { + t := s.Init(c) + + t.RunSubst( + "SUBST_CLASSES+= outer", + "SUBST_STAGE.outer= post-configure", + "SUBST_FILES.outer= outer.txt", + ".if ${OPSYS} == NetBSD", + "SUBST_CLASSES+= inner", + "SUBST_STAGE.inner= post-configure", + "SUBST_FILES.inner= inner.txt", + "SUBST_VARS.inner= INNER", + ".endif", + "SUBST_VARS.outer= OUTER") + + t.CheckOutputLines( + "WARN: filename.mk:5: Subst block \"outer\" should be finished " + + "before adding the next class to SUBST_CLASSES.") +} + +// It's completely valid to have several SUBST blocks in a single paragraph. +// As soon as a SUBST_CLASSES line appears, pkglint assumes that all previous +// SUBST blocks are finished. That's exactly the case here. +func (s *Suite) Test_SubstContext_directive__conditionally_following_block(c *check.C) { + t := s.Init(c) + + t.RunSubst( + "SUBST_CLASSES+= outer", + "SUBST_STAGE.outer= post-configure", + "SUBST_FILES.outer= outer.txt", + "SUBST_VARS.outer= OUTER", + ".if ${OPSYS} == NetBSD", + "SUBST_CLASSES+= middle", + "SUBST_STAGE.middle= post-configure", + "SUBST_FILES.middle= inner.txt", + "SUBST_VARS.middle= INNER", + ". if ${MACHINE_ARCH} == amd64", + "SUBST_CLASSES+= inner", + "SUBST_STAGE.inner= post-configure", + "SUBST_FILES.inner= inner.txt", + "SUBST_VARS.inner= INNER", + ". endif", + ".endif") + + t.CheckOutputEmpty() +} + +func (s *Suite) Test_SubstContext_directive__two_blocks_in_condition(c *check.C) { + t := s.Init(c) + + t.RunSubst( + ".if ${OPSYS} == NetBSD", + "SUBST_CLASSES+= a", + "SUBST_STAGE.a= post-configure", + "SUBST_FILES.a= outer.txt", + "SUBST_VARS.a= OUTER", + "SUBST_CLASSES+= b", + "SUBST_STAGE.b= post-configure", + "SUBST_FILES.b= inner.txt", + "SUBST_VARS.b= INNER", + ".endif") + + // Up to 2019-12-12, pkglint wrongly warned in filename.mk:6: + // Subst block "a" should be finished before adding + // the next class to SUBST_CLASSES. + // The warning was wrong since block "a" has all required fields set. + // The warning was caused by an inconsistent check whether the current + // block had any conditional variables. + t.CheckOutputEmpty() +} + +func (s *Suite) Test_SubstContext_directive__nested_conditional_incomplete_block(c *check.C) { + t := s.Init(c) + + t.RunSubst( + "SUBST_CLASSES+= outer", + "SUBST_STAGE.outer= post-configure", + "SUBST_FILES.outer= outer.txt", + "SUBST_VARS.outer= OUTER", + ".if ${OPSYS} == NetBSD", + "SUBST_CLASSES+= inner1", + "SUBST_STAGE.inner1= post-configure", + "SUBST_VARS.inner1= INNER", + "SUBST_CLASSES+= inner2", + "SUBST_STAGE.inner2= post-configure", + "SUBST_FILES.inner2= inner.txt", + "SUBST_VARS.inner2= INNER", + ".endif") + + t.CheckOutputLines( + "WARN: filename.mk:9: Subst block \"inner1\" should be finished "+ + "before adding the next class to SUBST_CLASSES.", + "WARN: filename.mk:9: Incomplete SUBST block: SUBST_FILES.inner1 missing.") +} + +func (s *Suite) Test_SubstContext_leave__details_in_then_branch(c *check.C) { + t := s.Init(c) + + t.RunSubst( + "SUBST_CLASSES+= os", + ".if ${OPSYS} == NetBSD", + "SUBST_VARS.os= OPSYS", + "SUBST_SED.os= -e s,@OPSYS@,NetBSD,", + "SUBST_STAGE.os= post-configure", + "SUBST_MESSAGE.os= Guessing operating system", + "SUBST_FILES.os= guess-os.h", + ".endif") + + t.CheckOutputLines( + "WARN: filename.mk:5: SUBST_STAGE.os should not be defined conditionally.", + "WARN: filename.mk:6: SUBST_MESSAGE.os should not be defined conditionally.", + "WARN: filename.mk:EOF: Incomplete SUBST block: SUBST_STAGE.os missing.", + "WARN: filename.mk:EOF: Incomplete SUBST block: SUBST_FILES.os missing.", + "WARN: filename.mk:EOF: Incomplete SUBST block: "+ + "SUBST_SED.os, SUBST_VARS.os or SUBST_FILTER_CMD.os missing.") +} + +func (s *Suite) Test_SubstContext_leave__details_in_else_branch(c *check.C) { + t := s.Init(c) + + t.RunSubst( + "SUBST_CLASSES+= os", + ".if ${OPSYS} == NetBSD", + ".else", + "SUBST_VARS.os= OPSYS", + "SUBST_SED.os= -e s,@OPSYS@,NetBSD,", + "SUBST_STAGE.os= post-configure", + "SUBST_MESSAGE.os= Guessing operating system", + "SUBST_FILES.os= guess-os.h", + ".endif") + + t.CheckOutputLines( + "WARN: filename.mk:6: SUBST_STAGE.os should not be defined conditionally.", + "WARN: filename.mk:7: SUBST_MESSAGE.os should not be defined conditionally.", + "WARN: filename.mk:EOF: Incomplete SUBST block: SUBST_STAGE.os missing.", + "WARN: filename.mk:EOF: Incomplete SUBST block: SUBST_FILES.os missing.", + "WARN: filename.mk:EOF: Incomplete SUBST block: "+ + "SUBST_SED.os, SUBST_VARS.os or SUBST_FILTER_CMD.os missing.") +} + +func (s *Suite) Test_SubstContext_leave__empty_conditional_at_end(c *check.C) { + t := s.Init(c) + + t.RunSubst( + "SUBST_CLASSES+= os", + "SUBST_VARS.os= OPSYS", + "SUBST_SED.os= -e s,@OPSYS@,NetBSD,", + "SUBST_STAGE.os= post-configure", + "SUBST_MESSAGE.os= Guessing operating system", + "SUBST_FILES.os= guess-os.h", + ".if ${OPSYS} == NetBSD", + ".else", + ".endif") + + t.CheckOutputEmpty() +} + +func (s *Suite) Test_SubstContext_leave__missing_transformation_in_one_branch(c *check.C) { + t := s.Init(c) + + t.RunSubst( + "SUBST_CLASSES+= os", + "SUBST_STAGE.os= post-configure", + "SUBST_MESSAGE.os= Guessing operating system", + "SUBST_FILES.os= guess-os.h", + ".if ${OPSYS} == NetBSD", + "SUBST_FILES.os= -e s,@OpSYS@,NetBSD,", // A simple typo, this should be SUBST_SED. + ".elif ${OPSYS} == Darwin", + "SUBST_SED.os= -e s,@OPSYS@,Darwin1,", + "SUBST_SED.os= -e s,@OPSYS@,Darwin2,", + ".else", + "SUBST_VARS.os= OPSYS", + ".endif") + + t.CheckOutputLines( + "WARN: filename.mk:6: All but the first assignment "+ + "to \"SUBST_FILES.os\" should use the \"+=\" operator.", + "WARN: filename.mk:9: All but the first assignment "+ + "to \"SUBST_SED.os\" should use the \"+=\" operator.", + "WARN: filename.mk:EOF: Incomplete SUBST block: SUBST_SED.os, "+ + "SUBST_VARS.os or SUBST_FILTER_CMD.os missing.") +} + +func (s *Suite) Test_SubstContext_leave__nested_conditionals(c *check.C) { + t := s.Init(c) + + t.RunSubst( + "SUBST_CLASSES+= os", + "SUBST_STAGE.os= post-configure", + "SUBST_MESSAGE.os= Guessing operating system", + ".if ${OPSYS} == NetBSD", + "SUBST_FILES.os= guess-netbsd.h", + ". if ${ARCH} == i386", + "SUBST_FILTER_CMD.os= ${SED} -e s,@OPSYS,NetBSD-i386,", + ". elif ${ARCH} == x86_64", + "SUBST_VARS.os= OPSYS", + ". else", + "SUBST_SED.os= -e s,@OPSYS,NetBSD-unknown", + ". endif", + ".else", + // This branch omits SUBST_FILES. + "SUBST_SED.os= -e s,@OPSYS@,unknown,", + ".endif") + + t.CheckOutputLines( + "WARN: filename.mk:EOF: Incomplete SUBST block: SUBST_FILES.os missing.") +} + +// With every .if directive, a new scope is created, to properly +// keep track of the conditional level at which the SUBST classes +// are declared. +// +// The scopes are even created when there is no SUBST variable +// anywhere close. The conditionals must be tracked for determining +// the end of the scope for the SUBST_CLASSES IDs. +func (s *Suite) Test_substScope__conditionals(c *check.C) { + t := s.Init(c) + + ctx := NewSubstContext() + + line := func(text string) { + mkline := t.NewMkLine("filename.mk", 123, text) + ctx.Process(mkline) + } + verifyScopes := func(n int) { + t.CheckEquals(len(ctx.scopes), n) + } + + verifyScopes(1) + + line(".if 1") + verifyScopes(2) + + line(". if 1") + verifyScopes(3) + + line(". if 1") + verifyScopes(4) + + line(". elif 1") + verifyScopes(4) + + line(". else") + verifyScopes(4) + + line(". endif") + verifyScopes(3) + + line(". endif") + verifyScopes(2) + + line(".endif") + verifyScopes(1) + + // An unbalanced .endif must not lead to a panic. + line(".endif") + verifyScopes(1) + + ctx.Finish(NewLineEOF("filename.mk")) +} + +func (s *Suite) Test_substScope_prepareSubstClasses(c *check.C) { + t := s.Init(c) + + t.RunSubst( + "SUBST_CLASSES+= 1", + "SUBST_STAGE.1= post-configure", + ".if 0") + + // There's no need to warn about unbalanced conditionals + // since that is already done by MkLines.Check. + t.CheckOutputLines( + "WARN: filename.mk:EOF: Incomplete SUBST block: SUBST_FILES.1 missing.", + "WARN: filename.mk:EOF: Incomplete SUBST block: "+ + "SUBST_SED.1, SUBST_VARS.1 or SUBST_FILTER_CMD.1 missing.") +} + +func (s *Suite) Test_substScope_prepareSubstClasses__nested(c *check.C) { + t := s.Init(c) + + t.RunSubst( + "SUBST_CLASSES+= 1", + "SUBST_STAGE.1= post-configure", + ".if 0", + ".if 0", + "SUBST_CLASSES+= 2") + + t.CheckOutputLines( + "WARN: filename.mk:5: Subst block \"1\" should be finished "+ + "before adding the next class to SUBST_CLASSES.", + "WARN: filename.mk:EOF: Missing SUBST block for \"2\".", + "WARN: filename.mk:EOF: Incomplete SUBST block: SUBST_FILES.1 missing.", + "WARN: filename.mk:EOF: Incomplete SUBST block: "+ + "SUBST_SED.1, SUBST_VARS.1 or SUBST_FILTER_CMD.1 missing.") +} + +func (s *Suite) Test_substBlock_varassignStage__do_patch(c *check.C) { + t := s.Init(c) + + t.RunSubst( + "SUBST_CLASSES+=\tos", + "SUBST_STAGE.os=\tdo-patch", + "SUBST_FILES.os=\tguess-os.h", + "SUBST_SED.os=\t-e s,@OPSYS@,Darwin,") + + // No warning, since there is nothing to fix automatically. + // This case doesn't occur in practice anyway. + t.CheckOutputEmpty() +} + +func (s *Suite) Test_substBlock_varassignStage__pre_patch(c *check.C) { t := s.Init(c) t.Chdir(".") @@ -495,7 +846,7 @@ func (s *Suite) Test_SubstContext_varassignStage__pre_patch(c *check.C) { "SUBST_SED.os= -e s,@OPSYS@,Darwin,") } -func (s *Suite) Test_SubstContext_varassignStage__post_patch(c *check.C) { +func (s *Suite) Test_substBlock_varassignStage__post_patch(c *check.C) { t := s.Init(c) t.Chdir(".") @@ -518,12 +869,31 @@ func (s *Suite) Test_SubstContext_varassignStage__post_patch(c *check.C) { "SUBST_SED.os= -e s,@OPSYS@,Darwin,") } +func (s *Suite) Test_substBlock_varassignStage__empty_class(c *check.C) { + t := s.Init(c) + + t.Chdir(".") + + t.RunSubst( + "SUBST_CLASSES+= id", + "", + "SUBST_STAGE.= post-patch", + "SUBST_STAGE.id= post-configure", + "SUBST_FILES.id= files", + "SUBST_VARS.id= VAR", + "SUBST_VARS.= VAR") + + t.CheckOutputLines( + "ERROR: filename.mk:3: Invalid SUBST class \"\" in variable name.", + "ERROR: filename.mk:7: Invalid SUBST class \"\" in variable name.") +} + // As of December 2019, pkglint does not use token positions internally. // Instead it only does simple string replacement when autofixing things. // To avoid damaging anything, replacements are only done if they are // unambiguous. This is not the case here, since line 4 contains the // string "pre-patch" twice. -func (s *Suite) Test_SubstContext_varassignStage__ambiguous_replacement(c *check.C) { +func (s *Suite) Test_substBlock_varassignStage__ambiguous_replacement(c *check.C) { t := s.Init(c) t.Chdir(".") @@ -541,7 +911,7 @@ func (s *Suite) Test_SubstContext_varassignStage__ambiguous_replacement(c *check t.CheckEquals(t.File("filename.mk").IsFile(), false) } -func (s *Suite) Test_SubstContext_varassignStage__with_NO_CONFIGURE(c *check.C) { +func (s *Suite) Test_substBlock_varassignStage__with_NO_CONFIGURE(c *check.C) { t := s.Init(c) pkg := t.SetUpPackage("category/package", @@ -572,7 +942,7 @@ func (s *Suite) Test_SubstContext_varassignStage__with_NO_CONFIGURE(c *check.C) "when NO_CONFIGURE is set (in line 35).") } -func (s *Suite) Test_SubstContext_varassignStage__without_NO_CONFIGURE(c *check.C) { +func (s *Suite) Test_substBlock_varassignStage__without_NO_CONFIGURE(c *check.C) { t := s.Init(c) pkg := t.SetUpPackage("category/package", @@ -589,7 +959,7 @@ func (s *Suite) Test_SubstContext_varassignStage__without_NO_CONFIGURE(c *check. // Before 2019-12-12, pkglint wrongly warned about variables that were // not obviously SUBST variables, even if they were used later in SUBST_VARS. -func (s *Suite) Test_SubstContext_varassignVars__var_before_SUBST_VARS(c *check.C) { +func (s *Suite) Test_substBlock_varassignVars__var_before_SUBST_VARS(c *check.C) { t := s.Init(c) t.RunSubst( @@ -612,23 +982,19 @@ func (s *Suite) Test_SubstContext_varassignVars__var_before_SUBST_VARS(c *check. "WARN: filename.mk:4: Foreign variable \"FOREIGN\" in SUBST block.") } -func (s *Suite) Test_SubstContext_dupList__conditional_before_unconditional(c *check.C) { +func (s *Suite) Test_substBlock_varassignVars(c *check.C) { t := s.Init(c) t.RunSubst( - "SUBST_CLASSES+= os", - "SUBST_STAGE.os= post-configure", - ".if 1", - "SUBST_FILES.os= conditional", - ".endif", - "SUBST_FILES.os= unconditional", - "SUBST_VARS.os= OPSYS") + "SUBST_CLASSES+=\tos", + "SUBST_STAGE.os=\tpre-configure", + "SUBST_FILES.os=\tguess-os.h", + "SUBST_VARS.os=\tPREFIX VARBASE") - // TODO: Warn that the conditional line is overwritten. t.CheckOutputEmpty() } -func (s *Suite) Test_SubstContext_suggestSubstVars(c *check.C) { +func (s *Suite) Test_substBlock_suggestSubstVars(c *check.C) { t := s.Init(c) t.Chdir(".") @@ -752,7 +1118,7 @@ func (s *Suite) Test_SubstContext_suggestSubstVars(c *check.C) { // If the SUBST_CLASS identifier ends with a plus, the generated code must // use the correct assignment operator and be nicely formatted. -func (s *Suite) Test_SubstContext_suggestSubstVars__plus(c *check.C) { +func (s *Suite) Test_substBlock_suggestSubstVars__plus(c *check.C) { t := s.Init(c) t.Chdir(".") @@ -786,7 +1152,7 @@ func (s *Suite) Test_SubstContext_suggestSubstVars__plus(c *check.C) { // The last of the SUBST_SED variables is 15 characters wide. When SUBST_SED // is replaced with SUBST_VARS, this becomes 16 characters and therefore // requires the whole paragraph to be indented by one more tab. -func (s *Suite) Test_SubstContext_suggestSubstVars__autofix_realign_paragraph(c *check.C) { +func (s *Suite) Test_substBlock_suggestSubstVars__autofix_realign_paragraph(c *check.C) { t := s.Init(c) t.Chdir(".") @@ -818,7 +1184,7 @@ func (s *Suite) Test_SubstContext_suggestSubstVars__autofix_realign_paragraph(c "SUBST_VARS.pfx+= PREFIX") } -func (s *Suite) Test_SubstContext_suggestSubstVars__autofix_plus_sed(c *check.C) { +func (s *Suite) Test_substBlock_suggestSubstVars__autofix_plus_sed(c *check.C) { t := s.Init(c) t.Chdir(".") @@ -849,7 +1215,7 @@ func (s *Suite) Test_SubstContext_suggestSubstVars__autofix_plus_sed(c *check.C) "SUBST_SED.pfx+= -e s,@PREFIX@,other,g") } -func (s *Suite) Test_SubstContext_suggestSubstVars__autofix_plus_vars(c *check.C) { +func (s *Suite) Test_substBlock_suggestSubstVars__autofix_plus_vars(c *check.C) { t := s.Init(c) t.Chdir(".") @@ -881,7 +1247,7 @@ func (s *Suite) Test_SubstContext_suggestSubstVars__autofix_plus_vars(c *check.C "SUBST_VARS.id+= PKGMANDIR") } -func (s *Suite) Test_SubstContext_suggestSubstVars__autofix_indentation(c *check.C) { +func (s *Suite) Test_substBlock_suggestSubstVars__autofix_indentation(c *check.C) { t := s.Init(c) t.Chdir(".") @@ -911,7 +1277,7 @@ func (s *Suite) Test_SubstContext_suggestSubstVars__autofix_indentation(c *check "SUBST_VARS.fix-paths= PREFIX") } -func (s *Suite) Test_SubstContext_suggestSubstVars__conditional(c *check.C) { +func (s *Suite) Test_substBlock_suggestSubstVars__conditional(c *check.C) { t := s.Init(c) t.Chdir(".") @@ -947,11 +1313,46 @@ func (s *Suite) Test_SubstContext_suggestSubstVars__conditional(c *check.C) { ".endif") } -func (s *Suite) Test_SubstContext_extractVarname(c *check.C) { +func (s *Suite) Test_substBlock_dupList__late_addition(c *check.C) { + t := s.Init(c) + + t.RunSubst( + "SUBST_CLASSES+=\tid", + "SUBST_STAGE.id=\tpost-configure", + "SUBST_FILES.id=\tfiles", + "SUBST_VARS.id=\tPREFIX", + "", + ".if ${OPSYS} == NetBSD", + "SUBST_VARS.id=\tOPSYS", + ".endif") + + t.CheckOutputLines( + "WARN: filename.mk:7: All but the first assignment " + + "to \"SUBST_VARS.id\" should use the \"+=\" operator.") +} + +func (s *Suite) Test_substBlock_dupList__conditional_before_unconditional(c *check.C) { + t := s.Init(c) + + t.RunSubst( + "SUBST_CLASSES+= os", + "SUBST_STAGE.os= post-configure", + ".if 1", + "SUBST_FILES.os= conditional", + ".endif", + "SUBST_FILES.os= unconditional", + "SUBST_VARS.os= OPSYS") + + t.CheckOutputLines( + "WARN: filename.mk:6: All but the first assignment " + + "to \"SUBST_FILES.os\" should use the \"+=\" operator.") +} + +func (s *Suite) Test_substBlock_extractVarname(c *check.C) { t := s.Init(c) test := func(input, expected string) { - t.CheckEquals((*SubstContext).extractVarname(nil, input), expected) + t.CheckEquals((*substBlock).extractVarname(nil, input), expected) } // A simple variable name. @@ -994,361 +1395,158 @@ func (s *Suite) Test_SubstContext_extractVarname(c *check.C) { test("s,@VAR@,${VAR}suffix,", "") } -func (s *Suite) Test_SubstContext_directive__before_SUBST_CLASSES(c *check.C) { +func (s *Suite) Test_substBlock_isComplete__incomplete(c *check.C) { t := s.Init(c) - t.RunSubst( - ".if 0", - ".endif", - "SUBST_CLASSES+=\tos", - ".elif 0") // Just for branch coverage. - - t.CheckOutputLines( - "WARN: filename.mk:EOF: Missing SUBST block for \"os\".") -} - -func (s *Suite) Test_SubstContext_directive__conditional_blocks_complete(c *check.C) { - t := s.Init(c) - - t.RunSubst( - ".if ${OPSYS} == NetBSD", - "SUBST_CLASSES+= nb", - "SUBST_STAGE.nb= post-configure", - "SUBST_FILES.nb= guess-netbsd.h", - "SUBST_VARS.nb= HAVE_NETBSD", - ".else", - "SUBST_CLASSES+= os", - "SUBST_STAGE.os= post-configure", - "SUBST_FILES.os= guess-netbsd.h", - "SUBST_VARS.os= HAVE_OTHER", - ".endif") + ctx := NewSubstContext() - t.CheckOutputEmpty() -} + ctx.varassign(t.NewMkLine("filename.mk", 10, "PKGNAME=pkgname-1.0")) -func (s *Suite) Test_SubstContext_directive__conditional_blocks_incomplete(c *check.C) { - t := s.Init(c) + t.CheckEquals(ctx.isActive(), false) - t.RunSubst( - ".if ${OPSYS} == NetBSD", - "SUBST_CLASSES+= nb", - "SUBST_STAGE.nb= post-configure", - "SUBST_VARS.nb= HAVE_NETBSD", - ".else", - "SUBST_CLASSES+= os", - "SUBST_STAGE.os= post-configure", - "SUBST_FILES.os= guess-netbsd.h", - ".endif") + ctx.varassign(t.NewMkLine("filename.mk", 11, "SUBST_CLASSES+=interp")) - t.CheckOutputLines( - "WARN: filename.mk:5: Incomplete SUBST block: SUBST_FILES.nb missing.", - "WARN: filename.mk:9: Incomplete SUBST block: "+ - "SUBST_SED.os, SUBST_VARS.os or SUBST_FILTER_CMD.os missing.") -} + t.CheckEquals(ctx.isActive(), false) -func (s *Suite) Test_SubstContext_directive__conditional_complete(c *check.C) { - t := s.Init(c) + ctx.varassign(t.NewMkLine("filename.mk", 12, "SUBST_FILES.interp=Makefile")) - t.RunSubst( - "SUBST_CLASSES+= id", - ".if ${OPSYS} == NetBSD", - "SUBST_STAGE.id=\t\tpost-configure", - "SUBST_MESSAGE.id=\tpost-configure", - "SUBST_FILES.id=\t\tguess-netbsd.h", - "SUBST_SED.id=\t\t-e s,from,to,", - "SUBST_VARS.id=\t\tHAVE_OTHER", - "SUBST_FILTER_CMD.id=\tHAVE_OTHER", - ".else", - "SUBST_STAGE.id=\t\tpost-configure", - "SUBST_MESSAGE.id=\tpost-configure", - "SUBST_FILES.id=\t\tguess-netbsd.h", - "SUBST_SED.id=\t\t-e s,from,to,", - "SUBST_VARS.id=\t\tHAVE_OTHER", - "SUBST_FILTER_CMD.id=\tHAVE_OTHER", - ".endif") + t.CheckEquals(ctx.activeId(), "interp") + t.CheckEquals(ctx.block().isComplete(), false) - t.CheckOutputLines( - "WARN: filename.mk:3: SUBST_STAGE.id should not be defined conditionally.", - "WARN: filename.mk:4: SUBST_MESSAGE.id should not be defined conditionally.", - "WARN: filename.mk:10: SUBST_STAGE.id should not be defined conditionally.", - "WARN: filename.mk:11: SUBST_MESSAGE.id should not be defined conditionally.") -} + ctx.varassign(t.NewMkLine("filename.mk", 13, "SUBST_SED.interp=s,@PREFIX@,${PREFIX},g")) -func (s *Suite) Test_SubstContext_directive__conditionally_overwritten_filter(c *check.C) { - t := s.Init(c) + t.CheckEquals(ctx.block().isComplete(), false) - t.RunSubst( - "SUBST_CLASSES+= id", - "SUBST_STAGE.id=\t\tpost-configure", - "SUBST_MESSAGE.id=\tpost-configure", - "SUBST_FILES.id=\t\tguess-netbsd.h", - "SUBST_FILTER_CMD.id=\tHAVE_OTHER", - ".if ${OPSYS} == NetBSD", - "SUBST_FILTER_CMD.id=\tHAVE_OTHER", - ".endif") + ctx.Finish(t.NewMkLine("filename.mk", 14, "")) t.CheckOutputLines( - "WARN: filename.mk:7: Duplicate definition of \"SUBST_FILTER_CMD.id\".") + "NOTE: filename.mk:13: The substitution command \"s,@PREFIX@,${PREFIX},g\" "+ + "can be replaced with \"SUBST_VARS.interp= PREFIX\".", + "WARN: filename.mk:14: Incomplete SUBST block: SUBST_STAGE.interp missing.") } -// Hopefully nobody will ever trigger this case in real pkgsrc. -// It's plain confusing to a casual reader to nest a complete -// SUBST block into another SUBST block. -// That's why pkglint doesn't cover this case correctly. -func (s *Suite) Test_SubstContext_directive__conditionally_nested_block(c *check.C) { +func (s *Suite) Test_substBlock_isComplete__complete(c *check.C) { t := s.Init(c) - t.RunSubst( - "SUBST_CLASSES+= outer", - "SUBST_STAGE.outer= post-configure", - "SUBST_FILES.outer= outer.txt", - ".if ${OPSYS} == NetBSD", - "SUBST_CLASSES+= inner", - "SUBST_STAGE.inner= post-configure", - "SUBST_FILES.inner= inner.txt", - "SUBST_VARS.inner= INNER", - ".endif", - "SUBST_VARS.outer= OUTER") - - t.CheckOutputLines( - "WARN: filename.mk:5: Incomplete SUBST block: "+ - "SUBST_SED.outer, SUBST_VARS.outer or SUBST_FILTER_CMD.outer missing.", - "WARN: filename.mk:5: Subst block \"outer\" should be finished "+ - "before adding the next class to SUBST_CLASSES.", - "WARN: filename.mk:10: "+ - "Late additions to a SUBST variable should use the += operator.") -} + ctx := NewSubstContext() -// It's completely valid to have several SUBST blocks in a single paragraph. -// As soon as a SUBST_CLASSES line appears, pkglint assumes that all previous -// SUBST blocks are finished. That's exactly the case here. -func (s *Suite) Test_SubstContext_directive__conditionally_following_block(c *check.C) { - t := s.Init(c) + ctx.varassign(t.NewMkLine("filename.mk", 10, "PKGNAME=pkgname-1.0")) + ctx.varassign(t.NewMkLine("filename.mk", 11, "SUBST_CLASSES+=p")) + ctx.varassign(t.NewMkLine("filename.mk", 12, "SUBST_FILES.p=Makefile")) + ctx.varassign(t.NewMkLine("filename.mk", 13, "SUBST_SED.p=s,@PREFIX@,${PREFIX},g")) - t.RunSubst( - "SUBST_CLASSES+= outer", - "SUBST_STAGE.outer= post-configure", - "SUBST_FILES.outer= outer.txt", - "SUBST_VARS.outer= OUTER", - ".if ${OPSYS} == NetBSD", - "SUBST_CLASSES+= middle", - "SUBST_STAGE.middle= post-configure", - "SUBST_FILES.middle= inner.txt", - "SUBST_VARS.middle= INNER", - ". if ${MACHINE_ARCH} == amd64", - "SUBST_CLASSES+= inner", - "SUBST_STAGE.inner= post-configure", - "SUBST_FILES.inner= inner.txt", - "SUBST_VARS.inner= INNER", - ". endif", - ".endif") + t.CheckEquals(ctx.block().isComplete(), false) - t.CheckOutputEmpty() -} + ctx.varassign(t.NewMkLine("filename.mk", 14, "SUBST_STAGE.p=post-configure")) -func (s *Suite) Test_SubstContext_directive__two_blocks_in_condition(c *check.C) { - t := s.Init(c) + t.CheckEquals(ctx.block().isComplete(), true) - t.RunSubst( - ".if ${OPSYS} == NetBSD", - "SUBST_CLASSES+= a", - "SUBST_STAGE.a= post-configure", - "SUBST_FILES.a= outer.txt", - "SUBST_VARS.a= OUTER", - "SUBST_CLASSES+= b", - "SUBST_STAGE.b= post-configure", - "SUBST_FILES.b= inner.txt", - "SUBST_VARS.b= INNER", - ".endif") + ctx.Finish(t.NewMkLine("filename.mk", 15, "")) - // Up to 2019-12-12, pkglint wrongly warned in filename.mk:6: - // Subst block "a" should be finished before adding - // the next class to SUBST_CLASSES. - // The warning was wrong since block "a" has all required fields set. - // The warning was caused by an inconsistent check whether the current - // block had any conditional variables. - t.CheckOutputEmpty() + t.CheckOutputLines( + "NOTE: filename.mk:13: The substitution command \"s,@PREFIX@,${PREFIX},g\" " + + "can be replaced with \"SUBST_VARS.p= PREFIX\".") } -func (s *Suite) Test_SubstContext_directive__nested_conditional_incomplete_block(c *check.C) { +func (s *Suite) Test_substBlock_finish__conditional_inside_then(c *check.C) { t := s.Init(c) t.RunSubst( - "SUBST_CLASSES+= outer", - "SUBST_STAGE.outer= post-configure", - "SUBST_FILES.outer= outer.txt", - "SUBST_VARS.outer= OUTER", - ".if ${OPSYS} == NetBSD", - "SUBST_CLASSES+= inner1", - "SUBST_STAGE.inner1= post-configure", - "SUBST_VARS.inner1= INNER", - "SUBST_CLASSES+= inner2", - "SUBST_STAGE.inner2= post-configure", - "SUBST_FILES.inner2= inner.txt", - "SUBST_VARS.inner2= INNER", + ".if ${OPSYS} == Linux", + "SUBST_CLASSES+=\tid", + "SUBST_STAGE.id=\tpre-configure", + "SUBST_SED.id=\t-e sahara", + ".else", ".endif") + // The block already ends at the .else, not at the end of the file, + // since that is the scope where the SUBST id is defined. t.CheckOutputLines( - "WARN: filename.mk:9: Incomplete SUBST block: SUBST_FILES.inner1 missing.", - "WARN: filename.mk:9: Subst block \"inner1\" should be finished "+ - "before adding the next class to SUBST_CLASSES.") + "WARN: filename.mk:5: Incomplete SUBST block: SUBST_FILES.id missing.") } -func (s *Suite) Test_SubstContext_finishClass__details_in_then_branch(c *check.C) { +func (s *Suite) Test_substBlock_finish__conditional_inside_else(c *check.C) { t := s.Init(c) t.RunSubst( - "SUBST_CLASSES+= os", - ".if ${OPSYS} == NetBSD", - "SUBST_VARS.os= OPSYS", - "SUBST_SED.os= -e s,@OPSYS@,NetBSD,", - "SUBST_STAGE.os= post-configure", - "SUBST_MESSAGE.os= Guessing operating system", - "SUBST_FILES.os= guess-os.h", + ".if ${OPSYS} == Linux", + ".else", + "SUBST_CLASSES+=\tid", + "SUBST_STAGE.id=\tpre-configure", + "SUBST_SED.id=\t-e sahara", ".endif") + // The block already ends at the .endif, not at the end of the file, + // since that is the scope where the SUBST id is defined. t.CheckOutputLines( - "WARN: filename.mk:5: SUBST_STAGE.os should not be defined conditionally.", - "WARN: filename.mk:6: SUBST_MESSAGE.os should not be defined conditionally.", - "WARN: filename.mk:EOF: Missing SUBST block for \"os\".") + "WARN: filename.mk:6: Incomplete SUBST block: SUBST_FILES.id missing.") } -func (s *Suite) Test_SubstContext_finishClass__details_in_else_branch(c *check.C) { +func (s *Suite) Test_substBlock_finish__files_missing(c *check.C) { t := s.Init(c) t.RunSubst( - "SUBST_CLASSES+= os", - ".if ${OPSYS} == NetBSD", - ".else", - "SUBST_VARS.os= OPSYS", - "SUBST_SED.os= -e s,@OPSYS@,NetBSD,", - "SUBST_STAGE.os= post-configure", - "SUBST_MESSAGE.os= Guessing operating system", - "SUBST_FILES.os= guess-os.h", - ".endif") + "SUBST_CLASSES+= one", + "SUBST_STAGE.one= pre-configure", + "SUBST_CLASSES+= two", + "SUBST_STAGE.two= pre-configure", + "SUBST_FILES.two= two.txt", + "SUBST_SED.two= s,two,2,g") t.CheckOutputLines( - "WARN: filename.mk:6: SUBST_STAGE.os should not be defined conditionally.", - "WARN: filename.mk:7: SUBST_MESSAGE.os should not be defined conditionally.", - "WARN: filename.mk:EOF: Missing SUBST block for \"os\".") + "WARN: filename.mk:3: Subst block \"one\" should be finished "+ + "before adding the next class to SUBST_CLASSES.", + "WARN: filename.mk:3: Incomplete SUBST block: SUBST_FILES.one missing.", + "WARN: filename.mk:3: Incomplete SUBST block: "+ + "SUBST_SED.one, SUBST_VARS.one or SUBST_FILTER_CMD.one missing.") } -func (s *Suite) Test_SubstContext_finishClass__empty_conditional_at_end(c *check.C) { +// Variables mentioned in SUBST_VARS may appear in the same paragraph, +// or alternatively anywhere else in the file. +func (s *Suite) Test_substBlock_checkForeignVariables__in_next_paragraph(c *check.C) { t := s.Init(c) t.RunSubst( - "SUBST_CLASSES+= os", - "SUBST_VARS.os= OPSYS", - "SUBST_SED.os= -e s,@OPSYS@,NetBSD,", - "SUBST_STAGE.os= post-configure", - "SUBST_MESSAGE.os= Guessing operating system", - "SUBST_FILES.os= guess-os.h", - ".if ${OPSYS} == NetBSD", - ".else", - ".endif") + "SUBST_CLASSES+=\tos", + "SUBST_STAGE.os=\tpre-configure", + "SUBST_FILES.os=\tguess-os.h", + "SUBST_VARS.os=\tTODAY1", + "", + "TODAY1!=\tdate", + "TODAY2!=\tdate") t.CheckOutputEmpty() } -func (s *Suite) Test_SubstContext_finishClass__missing_transformation_in_one_branch(c *check.C) { +func (s *Suite) Test_substBlock_checkForeignVariables__mixed_separate(c *check.C) { t := s.Init(c) t.RunSubst( - "SUBST_CLASSES+= os", - "SUBST_STAGE.os= post-configure", - "SUBST_MESSAGE.os= Guessing operating system", - "SUBST_FILES.os= guess-os.h", - ".if ${OPSYS} == NetBSD", - "SUBST_FILES.os= -e s,@OpSYS@,NetBSD,", // A simple typo, this should be SUBST_SED. - ".elif ${OPSYS} == Darwin", - "SUBST_SED.os= -e s,@OPSYS@,Darwin1,", - "SUBST_SED.os= -e s,@OPSYS@,Darwin2,", - ".else", - "SUBST_VARS.os= OPSYS", - ".endif") + "SUBST_CLASSES+= 1", + "SUBST_STAGE.1= post-configure", + "SUBST_FILES.1= files", + "", + "SUBST_VARS.1= VAR", + "USE_TOOLS+= gmake") - t.CheckOutputLines( - "WARN: filename.mk:6: All but the first assignment "+ - "to \"SUBST_FILES.os\" should use the \"+=\" operator.", - "WARN: filename.mk:9: All but the first assignment "+ - "to \"SUBST_SED.os\" should use the \"+=\" operator.", - "WARN: filename.mk:EOF: Incomplete SUBST block: SUBST_SED.os, "+ - "SUBST_VARS.os or SUBST_FILTER_CMD.os missing.") + // The USE_TOOLS is not in the SUBST block anymore since there is + // an empty line between SUBST_CLASSES and SUBST_VARS. + t.CheckOutputEmpty() } -func (s *Suite) Test_SubstContext_finishClass__nested_conditionals(c *check.C) { +// Variables mentioned in SUBST_VARS are not considered "foreign" +// in the block and may be mixed with the other SUBST variables. +func (s *Suite) Test_substBlock_checkForeignVariables__in_block(c *check.C) { t := s.Init(c) t.RunSubst( - "SUBST_CLASSES+= os", - "SUBST_STAGE.os= post-configure", - "SUBST_MESSAGE.os= Guessing operating system", - ".if ${OPSYS} == NetBSD", - "SUBST_FILES.os= guess-netbsd.h", - ". if ${ARCH} == i386", - "SUBST_FILTER_CMD.os= ${SED} -e s,@OPSYS,NetBSD-i386,", - ". elif ${ARCH} == x86_64", - "SUBST_VARS.os= OPSYS", - ". else", - "SUBST_SED.os= -e s,@OPSYS,NetBSD-unknown", - ". endif", - ".else", - // This branch omits SUBST_FILES. - "SUBST_SED.os= -e s,@OPSYS@,unknown,", - ".endif") - - t.CheckOutputLines( - "WARN: filename.mk:EOF: Incomplete SUBST block: SUBST_FILES.os missing.") -} - -func (s *Suite) Test_SubstContext_isComplete__incomplete(c *check.C) { - t := s.Init(c) - - ctx := NewSubstContext() - - ctx.varassign(t.NewMkLine("filename.mk", 10, "PKGNAME=pkgname-1.0")) - - t.CheckEquals(ctx.id, "") - - ctx.varassign(t.NewMkLine("filename.mk", 11, "SUBST_CLASSES+=interp")) - - t.CheckEquals(ctx.id, "interp") - - ctx.varassign(t.NewMkLine("filename.mk", 12, "SUBST_FILES.interp=Makefile")) - - t.CheckEquals(ctx.isComplete(), false) - - ctx.varassign(t.NewMkLine("filename.mk", 13, "SUBST_SED.interp=s,@PREFIX@,${PREFIX},g")) - - t.CheckEquals(ctx.isComplete(), false) - - ctx.Finish(t.NewMkLine("filename.mk", 14, "")) - - t.CheckOutputLines( - "NOTE: filename.mk:13: The substitution command \"s,@PREFIX@,${PREFIX},g\" "+ - "can be replaced with \"SUBST_VARS.interp= PREFIX\".", - "WARN: filename.mk:14: Incomplete SUBST block: SUBST_STAGE.interp missing.") -} - -func (s *Suite) Test_SubstContext_isComplete__complete(c *check.C) { - t := s.Init(c) - - ctx := NewSubstContext() - - ctx.varassign(t.NewMkLine("filename.mk", 10, "PKGNAME=pkgname-1.0")) - ctx.varassign(t.NewMkLine("filename.mk", 11, "SUBST_CLASSES+=p")) - ctx.varassign(t.NewMkLine("filename.mk", 12, "SUBST_FILES.p=Makefile")) - ctx.varassign(t.NewMkLine("filename.mk", 13, "SUBST_SED.p=s,@PREFIX@,${PREFIX},g")) - - t.CheckEquals(ctx.isComplete(), false) - - ctx.varassign(t.NewMkLine("filename.mk", 14, "SUBST_STAGE.p=post-configure")) - - t.CheckEquals(ctx.isComplete(), true) - - ctx.Finish(t.NewMkLine("filename.mk", 15, "")) + "SUBST_CLASSES+=\tos", + "SUBST_STAGE.os=\tpre-configure", + "SUBST_FILES.os=\tguess-os.h", + "SUBST_VARS.os=\tTODAY1", + "TODAY1!=\tdate", + "TODAY2!=\tdate") t.CheckOutputLines( - "NOTE: filename.mk:13: The substitution command \"s,@PREFIX@,${PREFIX},g\" " + - "can be replaced with \"SUBST_VARS.p= PREFIX\".") + "WARN: filename.mk:6: Foreign variable \"TODAY2\" in SUBST block.") } |