pkgtools/R2pkg: code cleanup, use fewer temporary files

3 files changed, 60 insertions, 107 deletions

diff --git a/pkgtools/R2pkg/files/R2pkg.R b/pkgtools/R2pkg/files/R2pkg.R
index 207db88aa0d..d966ba8d687 100644
--- a/pkgtools/R2pkg/files/R2pkg.R
+++ b/pkgtools/R2pkg/files/R2pkg.R
@@ -1,4 +1,4 @@
-# $NetBSD: R2pkg.R,v 1.21 2019/10/19 17:30:10 rillig Exp $
+# $NetBSD: R2pkg.R,v 1.22 2019/10/19 18:43:51 rillig Exp $
 #
 # Copyright (c) 2014,2015,2016,2017,2018,2019
 #	Brook Milligan.  All rights reserved.
@@ -174,17 +174,20 @@ as.sorted.list <- function(df)
 
 read.file.as.dataframe <- function(filename)
 {
-  # message('===> read.file.as.dataframe(',filename,')')
-  contents <- as.list(readLines(filename))
   df <- data.frame()
-  for (line in contents)
-    {
-      # str(line)
-      df <- rbind(df,data.frame(line=line,stringsAsFactors=FALSE))
-    }
+  for (line in as.list(readLines(filename)))
+    df <- rbind(df, data.frame(line = line, stringsAsFactors = FALSE))
   df
 }
 
+mklines.get_value <- function(mklines, varname, default = '')
+{
+  values <- mklines$old_value[mklines$key == varname]
+  if (length(values) == 0)
+    values <- mklines$old_value[mklines$key == paste0('#', varname)]
+  if (length(values) == 1) values[1] else default
+}
+
 categorize.key_value <- function(df,line='line')
 {
   re.skip_blank <- '[[:blank:]]*'
@@ -261,7 +264,7 @@ fix.continued.lines <- function(df,line='line')
   df
 }
 
-read.Makefile.as.dataframe <- function(filename = 'Makefile.orig')
+read.Makefile.as.dataframe <- function(filename)
 {
   re_varassign <- paste0(
     '^',
@@ -305,34 +308,6 @@ read.file.as.list <- function(filename)
   result
 }
 
-read.file.as.value <- function(filename)
-{
-  value <- ''
-  l <- read.file.as.list(filename)
-  if (length(l) == 1)
-    {
-      line <- l[[1]]
-      fields <- strsplit(line,'[[:blank:]]+')
-      value <- fields[[1]][2]
-    }
-  value
-}
-
-read.file.as.values <- function(filename)
-{
-  message('===> read.file.as.values(',filename,'):')
-  values <- list()
-  l <- read.file.as.list(filename)
-  print(l)
-  for (line in l)
-    {
-      # fields <- strsplit(line,'[[:blank:]]+')
-      # value <- fields[[1]][2]
-    }
-  print(values)
-  values
-}
-
 simplify.whitespace <- function(s) gsub('[[:blank:]]+', ' ', s)
 remove.punctuation <- function(s) gsub('[,-]', '', s)
 remove.quotes <- function(s) gsub('[\'`"]', '', s)
@@ -352,29 +327,17 @@ weakly.equals <- function(s1,s2)
     remove.articles(remove.quotes(remove.punctuation(s2))))
 }
 
-pkgsrc.license <- function(s)
+license <- function(mklines, s)
 {
   license <- licenses[[s]]
-  if (is.null(license)) license <- s else license
-}
-
-license <- function(s)
-{
-  license <- pkgsrc.license(s)
-  old.license <- read.file.as.value('LICENSE')
+  if (is.null(license))
+    license <- s
+  old.license <- mklines.get_value(mklines, 'LICENSE')
   if (old.license != '' && old.license != license)
-    license <- paste0(license,'\t# [R2pkg] previously: ',old.license)
+    license <- paste0(license, '\t# [R2pkg] previously: ', old.license)
   license
 }
 
-maintainer <- function(email)
-{
-  MAINTAINER <- read.file.as.value('MAINTAINER')
-  if (MAINTAINER == '')
-    MAINTAINER <- email
-  MAINTAINER
-}
-
 find.Rcpp <- function(imps, deps)
   any(grepl('Rcpp', paste(imps, deps)))
 
@@ -602,11 +565,9 @@ make.depends <- function(imps,deps)
             system(paste('echo', depends(dependency), arg.rpkg, '>>', arg.dependency_list))
         }
     }
-  DEPENDS <- as.sorted.list(DEPENDS)
-  DEPENDS <- end.paragraph(DEPENDS)
+  DEPENDS <- end.paragraph(as.sorted.list(DEPENDS))
   BUILDLINK3.MK <- as.sorted.list(BUILDLINK3.MK)
-  result <- list(DEPENDS,BUILDLINK3.MK)
-  result
+  list(DEPENDS,BUILDLINK3.MK)
 }
 
 use_languages <- function(imps, deps)
@@ -618,35 +579,29 @@ copy.description <- function(connection)
   writeLines(description,con='DESCRIPTION')
 }
 
-write.Makefile <- function(metadata)
+write.Makefile <- function(orig_mklines, metadata)
 {
-  CATEGORIES    <- varassign('CATEGORIES', categories())
-  MAINTAINER    <- varassign('MAINTAINER', maintainer(arg.maintainer_email))
-  COMMENT       <- varassign('COMMENT', one.line(metadata$Title))
-  LICENSE       <- varassign('LICENSE', license(metadata$License))
-  R_PKGNAME     <- varassign('R_PKGNAME', one.line(metadata$Package))
-  R_PKGVER      <- varassign('R_PKGVER', one.line(metadata$Version))
-  USE_LANGUAGES <- varassign('USE_LANGUAGES', use_languages(metadata$Imports, metadata$Depends))
+  maintainer    <- mklines.get_value(orig_mklines, 'MAINTAINER', arg.maintainer_email)
+  license       <- license(orig_mklines, metadata$License)
+  use_languages <- use_languages(metadata$Imports, metadata$Depends)
   dependencies  <- make.depends(metadata$Imports, metadata$Depends)
-  depends       <- dependencies[1]
-  buildlink3    <- dependencies[2]
 
   lines <- c(
     mkcvsid,
     '',
-    R_PKGNAME,
-    R_PKGVER,
-    CATEGORIES,
+    varassign('R_PKGNAME', one.line(metadata$Package)),
+    varassign('R_PKGVER', one.line(metadata$Version)),
+    varassign('CATEGORIES', categories()),
     '',
-    MAINTAINER,
-    COMMENT,
-    LICENSE,
+    varassign('MAINTAINER', maintainer),
+    varassign('COMMENT', one.line(metadata$Title)),
+    varassign('LICENSE', license),
     '',
-    depends,
-    USE_LANGUAGES,
+    dependencies[1],
+    varassign('USE_LANGUAGES', use_languages),
     '',
     '.include "../../math/R/Makefile.extension"',
-    buildlink3,
+    dependencies[2],
     '.include "../../mk/bsd.pkg.mk"',
     recursive = TRUE)
 
@@ -954,7 +909,7 @@ make.df.makefile <- function(df,df.conflicts,df.depends,df.buildlink3.mk)
   df.makefile
 }
 
-update.Makefile <- function(metadata)
+update.Makefile <- function(orig, metadata)
 {
   DEPENDENCIES  <- make.depends(metadata$Imports,metadata$Depends)
   DEPENDS       <- DEPENDENCIES[[1]]
@@ -967,7 +922,7 @@ update.Makefile <- function(metadata)
   # print(BUILDLINK3.MK)
 
   # message('===> df:')
-  df <- read.Makefile.as.dataframe()
+  df <- orig
   df <- mklines.update_with_metadata(df, metadata)
   df <- mklines.update_value(df)
   df <- mklines.update_new_line(df)
@@ -985,10 +940,13 @@ update.Makefile <- function(metadata)
 
 create.Makefile <- function(metadata)
 {
-  if (arg.update && file.exists('Makefile.orig'))
-    update.Makefile(metadata)
-  else
-    write.Makefile(metadata)
+  if (arg.update && file.exists('Makefile.orig')) {
+    orig <- read.Makefile.as.dataframe('Makefile.orig')
+    update.Makefile(orig, metadata)
+  } else {
+    orig <- read.Makefile.as.dataframe(textConnection(''))
+    write.Makefile(orig, metadata)
+  }
 }
 
 create.DESCR <- function(metadata) {
diff --git a/pkgtools/R2pkg/files/R2pkg.sh b/pkgtools/R2pkg/files/R2pkg.sh
index edd5c678857..e7bb15ba66d 100755
--- a/pkgtools/R2pkg/files/R2pkg.sh
+++ b/pkgtools/R2pkg/files/R2pkg.sh
@@ -1,5 +1,5 @@
 #!/bin/sh
-# $NetBSD: R2pkg.sh,v 1.11 2019/10/19 17:15:00 rillig Exp $
+# $NetBSD: R2pkg.sh,v 1.12 2019/10/19 18:43:51 rillig Exp $
 #
 # Copyright (c) 2014,2015,2016,2017,2018,2019
 #	Brook Milligan.  All rights reserved.
@@ -147,8 +147,6 @@ preserve_original_content ()
 {
     [ -f Makefile ] && grep -e "CATEGORIES=" Makefile > CATEGORIES
     [ -f Makefile ] && grep -e "COMMENT=" Makefile > COMMENT
-    [ -f Makefile ] && grep -e "LICENSE=" Makefile > LICENSE
-    [ -f Makefile ] && grep -e "MAINTAINER=" Makefile > MAINTAINER
     [ -f Makefile ] && grep -e "USE_LANGUAGES" Makefile > USE_LANGUAGES
     [ -f Makefile ] && grep -e "USE_TOOLS" Makefile > USE_TOOLS
     [ -f Makefile ] && grep -e "DEPENDS" Makefile > DEPENDS
@@ -297,7 +295,6 @@ cleanup_misc_files ()
     [ "$keep_description" = "yes" ] || rm -f DESCRIPTION
     rm -f CATEGORIES
     rm -f COMMENT
-    rm -f MAINTAINER
     rm -f USE_LANGUAGES
     rm -f USE_TOOLS
     rm -f DEPENDS
diff --git a/pkgtools/R2pkg/files/R2pkg_test.R b/pkgtools/R2pkg/files/R2pkg_test.R
index 448d948e2ad..0907a3e7441 100644
--- a/pkgtools/R2pkg/files/R2pkg_test.R
+++ b/pkgtools/R2pkg/files/R2pkg_test.R
@@ -1,4 +1,4 @@
-# $NetBSD: R2pkg_test.R,v 1.16 2019/10/19 17:30:10 rillig Exp $
+# $NetBSD: R2pkg_test.R,v 1.17 2019/10/19 18:43:51 rillig Exp $
 #
 # Copyright (c) 2019
 #	Roland Illig.  All rights reserved.
@@ -278,43 +278,40 @@ test_that('read.file.as.list can read lines from a file', {
     expect_equal(lines, list('first', 'second \\', 'third'))
 })
 
-test_that('read.file.as.value, exactly 1 variable assignment, no space', {
-    filename <- ''
-    local_tempfile('filename')
-    writeLines(c('VAR=value'), filename)
+test_that('mklines.get_value, exactly 1 variable assignment, no space', {
+    mklines <- make_mklines(
+        'VAR=value')
 
-    str <- read.file.as.value(filename)
+    str <- mklines.get_value(mklines, 'VAR')
 
-    expect_equal(str, NA_character_)  # FIXME
+    expect_equal(str, 'value')
 })
 
 test_that('read.file.as.value, exactly 1 variable assignment', {
-    filename <- ''
-    local_tempfile('filename')
-    writeLines(c('VAR=\tvalue'), filename)
+    mklines <- make_mklines(
+        'VAR=\tvalue')
 
-    str <- read.file.as.value(filename)
+    str <- mklines.get_value(mklines, 'VAR')
 
     expect_equal(str, 'value')
 })
 
 test_that('read.file.as.value, commented variable assignment', {
-    filename <- ''
-    local_tempfile('filename')
-    writeLines(c('#VAR=\tvalue'), filename)
+    mklines <- make_mklines(
+        '#VAR=\tvalue')
 
-    str <- read.file.as.value(filename)
+    str <- mklines.get_value(mklines, 'VAR')
 
     # TODO: Check whether commented variables should really be considered.
     expect_equal(str, 'value')
 })
 
 test_that('read.file.as.value, multiple variable assignments', {
-    filename <- ''
-    local_tempfile('filename')
-    writeLines(c('VAR=\tvalue', 'VAR=\tvalue2'), filename)
+    mklines <- make_mklines(
+        'VAR=\tvalue',
+        'VAR=\tvalue2')
 
-    str <- read.file.as.value(filename)
+    str <- mklines.get_value(mklines, 'VAR')
 
     expect_equal(str, '')
 })
@@ -560,8 +557,9 @@ test_that('write.Makefile', {
         'Package: pkgname',
         'Version: 1.3',
         'Depends: ellipsis'))
+    orig <- make_mklines()
 
-    write.Makefile(metadata)
+    write.Makefile(orig, metadata)
 
     expect_equal(readLines('Makefile'),c(
         mkcvsid,