1. 16 Oct, 2018 1 commit
    • Luke Shumaker's avatar
      test: db-remove: Verify that it accepts pkgname (in addition to pkgbase) · 1bf2b35e
      Luke Shumaker authored and Eli Schwartz's avatar Eli Schwartz committed
      It is important that db-remove be able to remove a single pkgname, without
      being able to look it up by pkgbase in SVN.  For instance, when a split
      package update removes one of its members; there will be no reference to
      the removed pkgname in SVN, and it won't be removed by db-update.  If
      db-remove doesn't accept pkgnames, then this outdated orphan could not be
      removed.
      1bf2b35e
  2. 08 Oct, 2018 7 commits
    • Eli Schwartz's avatar
    • Eli Schwartz's avatar
    • Eli Schwartz's avatar
      Factor out the exporting of files/folders from svn. · 37a493d3
      Eli Schwartz authored
      As of the source_pkgbuild rewrite, this is only ever done once.
      37a493d3
    • Eli Schwartz's avatar
      Preliminary work to break out svn-specific code. · d6b48bd6
      Eli Schwartz authored
      Introduce "db-functions-$VCS" which will eventually contain all
      VCS-specific code, and make this configurable in config.
      
      Move private arch_svn function and svn acl handling here, and introduce
      a new source_pkgbuild function to handle discovering PKGBUILDs from the
      configured VCS and sourcing them to extract metadata.
      
      The PKGBUILD is the only file we ever check out from version control,
      and only ever to scrape information from it, except for when we actually
      want to db-move a whole directory (which is by necessity considerably
      dependent on the VCS in use).
      
      source_pkgbuild is inspired by commits from the dbscripts rewrite,
      authored by Florian Pritz <bluewind@xinu.at>
      d6b48bd6
    • Luke Shumaker's avatar
      test: checkPackageDB(): Resolve "FIXME: We guess the location of the PKGBUILD" · 971181bf
      Luke Shumaker authored and Eli Schwartz's avatar Eli Schwartz committed
      The problem statement:
      
        checkPackageDB and checkRemovedPackageDB need bit of information on the
        package they're checking: a full list of pkgnames with that pkgbase, the
        list of pkgarches, and (for checkPackageDB only) the full version.  In
        dbscripts itself, we can get that directly from the .db files; however,
        since the test suite is checking the ability of dbscripts to edit those
        .db files, that's obviously not a good solution.
      
        The current solution is to get this information from the PKGBUILD...
        which we also count on dbscripts to correctly keep track of.  Wait,
        that's skipping ahead, let's back up.
      
        The current solution is to get this information from the PKGBUILD.  For
        checkRemovedPackageDB, that's easy; we just get it from trunk, as that's
        the most up-to-date information on the package as-it-would-have-existed
        (if that sounded a little hand-wavey, it was).  But for checkPackageDB,
        it's a little trickier, because of 2 factors working together: (1) there
        might be different versions on different repos, and (2) unlike
        checkRemovedPackageDB, checkPackageDB actually cares about pkgver.  So,
        checkPackageDB "guesses" the location in a slightly sloppy way, and is
        tagged with a "FIXME".
      
      What todo about it?
      
        There are a couple of things to observe:
      
         - Hidden in the hand-waving in assuming that "trunk" is fine for
           checkRemovedPackageDB is the assumption that neither pkgname=() nor
           arch=() is going to change between versions.  Which is a fine
           assumption, because we don't change those things between versions in
           any of our test cases.
         - We're counting on dbscripts correctly keeping track of which PKGBUILD
           is correct for which repo... which is one of the thing's we're trying
           to test, we shouldn't be counting on it.  That's actually a bigger
           problem than the original "FIXME"!
      
        So, putting those things together, let's (1) take the code under test out
        of the equation, and (2) remove any suggestion that the version of the
        PKGBUILD makes a difference to pkgnames/pkgarches: Let's have both
        functions that that information from the PKGBUILDs under "fixtures/",
        rather than getting PKGBUILDs from VCS.
      
        That just leaves one question: How to get the correct pkgver in
        checkPackageDB?  The obvious answer is: Each test case knows what the
        version should be; add it as an argument, and adjust every testcase that
        calls it.
      971181bf
    • Luke Shumaker's avatar
      test: Don't use "! cmd" except as the last statement in a function · a75e4ee5
      Luke Shumaker authored and Eli Schwartz's avatar Eli Schwartz committed
      With BATS setting up error traps, we get used to writing simple `[[ foo ]]`
      assertions and not having to check the result.  However, using `!` to
      invert the result DOESN'T trigger the trap.  It works OK when it is the
      last command in a function, as it still affects the function's return
      value, so then the trap triggers in the caller (rather than in the
      function).  This means that a check may run successfully when it should
      fail.
      
      So, replace all uses of bare `! cmd` with `if cmd; then return 1; fi`,
      unless it is the final statement in a function (as it is in
      sourceballs.bats:__checkRemovedSourcePackage())
      
      The mistake meant that checkRemovedPackage() was effectively equivalent to
      checkRemovedPackageDB().  This meant that no one noticed that db-updates's
      "add package with inconsistent name fails" test called checkRemovedPackage
      instead of checkRemovedPackageDB.  Fixing the ! issue means that the test
      now fails, so change which function it calls.
      a75e4ee5
    • Luke Shumaker's avatar
      test: db-move: Verify that it works on single-arch packages · e712811b
      Luke Shumaker authored and Eli Schwartz's avatar Eli Schwartz committed
      e712811b
  3. 09 Sep, 2018 2 commits
    • Levente Polyak's avatar
      fix potential bsdtar stream close error by grep · 0b630e25
      Levente Polyak authored and Eli Schwartz's avatar Eli Schwartz committed
      This silences a useless error message that confuses the user.
      
      bsdtar doesn't like it when the stream gets closed before it finishes
      which may be the case when grep found its match on potentially huge
      archives. Instead of suppressing the whole stderr , we find all matches
      with grep, then use a second pass with `tail` to find only the last
      match, which ensures the stream remains open for bsdtar but we may still
      catch and see useful messages on stderr.
      
      This works because tail has the useful property of not closing early.
      0b630e25
    • Levente Polyak's avatar
      readme: switch to travis-ci.com build status badge · 57a307d6
      Levente Polyak authored and Eli Schwartz's avatar Eli Schwartz committed
      The old travis-ci.org is deprecated and this project was migrated.
      57a307d6
  4. 26 Aug, 2018 1 commit
  5. 04 Jul, 2018 1 commit
  6. 27 Jun, 2018 2 commits
  7. 19 Jun, 2018 1 commit
    • Eli Schwartz's avatar
      test: BUILDDIR must be owned by build user · 05dd9be0
      Eli Schwartz authored
      pacman 5.1 enforces this restriction. OTOH it is a simpler setup to set
      this as the user homedir directly in account creation (just like
      makechrootpkg has always done) than to create an additional,
      world-writable, directory.
      
      dockerfile: don't use tmpfs for /build
      05dd9be0
  8. 28 May, 2018 1 commit
  9. 08 Apr, 2018 4 commits
  10. 02 Apr, 2018 2 commits
  11. 21 Mar, 2018 2 commits
    • Luke Shumaker's avatar
      Clean up printf-formatters for user messages · 8cc8e9cb
      Luke Shumaker authored and Eli Schwartz's avatar Eli Schwartz committed
       - db-functions: getpkgfile: The .sig file extension should be part of
         the filename parameter, not part of the message format string.
      
       - db-functions: arch_repo_modify: Shouldn't use ${action}
         string-interpolation in the message format string.  Since the
         entire message is a command, and we're using @Q to escape arguments
         anyway, go ahead and just construct the entire command as a single
         string that way, and feed it to '%s'.
      8cc8e9cb
    • Luke Shumaker's avatar
      Fixups near unquoted variables · 0432cffc
      Luke Shumaker authored and Eli Schwartz's avatar Eli Schwartz committed
      Using the following command to find unquoted variables (and ignoring
      more than a a few false positives),
      
          grep -Prn --exclude-dir=.git '(?<!["=]|\[\[ |\[\[ -[zn] )\$(?!{?#|\(|\? )'
      
      one is lead to find a few cleanups that are something other than "add
      double-quotes".  That's what these are.  We'll leave dumb adding of
      double-quotes for another commit.
      
      Most of these are still fixing quoting issues, just with a better fix.
      
       - parse_pkgbuilds.sh: Avoid having to escape quotes in `eval` strings
         by using `declare -p`. Updates the logic copied from makepkg, with the
         latest logic copied from makepkg. See
         https://git.archlinux.org/pacman.git/commit/?id=9e52a36794552b77ecf26f7f34b226d096978f1e
       - sourceballs: Avoid using ary=($string) to do field separation by
         using `read` and test that multiple licenses actually work as
         expected.
       - sourceballs: Replace `[[ -z ${ary[*]} ]]` with test for the array
         length
       - db-functions: Replace mangling echo field separators using sed, with
         printf formatters
       - db-functions: Replace for/echo loop to print an array line by line,
         with `printf '%s\n'`
       - db-functions: set_repo_permissions: Line up error messages, quote
         "$group"
       - db-move: Replace `$(echo ${array[@]})` with `${array[*]}`
       - testing2x: Use `"$@"` instead of `$*` when looping over an array
      
      Also, not really quoting related but on the same line as a quoting
      issue, optimize:
      
       - db-functions: Replace
      	[[ -n "$(... | sort | uniq -D)" ]]
         with
      	! ... | awk 'a[$0]++{exit 1}'
      0432cffc
  12. 20 Mar, 2018 5 commits
    • Luke Shumaker's avatar
      Remove uses of the "v=true; if $v ..." anti-pattern · 36087fbd
      Luke Shumaker authored and Eli Schwartz's avatar Eli Schwartz committed
      Instead, compare the value of $v to 'true'.
      36087fbd
    • Luke Shumaker's avatar
      Don't use `grep -q` when operating on piped stdin · 23c2b82c
      Luke Shumaker authored and Eli Schwartz's avatar Eli Schwartz committed
      (By default, prefer `grep &>/dev/null`)
      
      `grep -q` may exit as soon as it finds a match; this is a good optimization
      for when the input is a file.  However, if the input is the output of
      another program, then that other program will receive SIGPIPE, and further
      writes will fail.  When this happens, it might (bsdtar does) print a
      message about a "write error" to stderr.  Which is going to confuse and
      alarm the user.
      
      In one of the cases (in common.bash, in the test suite), this had
      already been mitigated by wrapping bsdtar in "echo "$(bsdtar ...)", as
      Bash builtin echo doesn't complain if it gets SIGPIPE.  However, that
      means we're storing the entire output of bsdtar in memory, which is
      silly.  Additionally, the way it was implemented is also wrong;
      because it was being used with `grep -qv` instead of just `grep -q`,
      it *always* found a non-matching line (even something inconsequential
      like `%NAME%`), and *never* triggered a test failure.
      
      Looking at a few of these cases, it might also make sense to switch to
      using `bsdtar tf` instead of `bsdtar xf` when checking membership, but
      that's work for another day.
      23c2b82c
    • Luke Shumaker's avatar
      Add "#!/hint/bash" to the beginning of several files · 97e17a59
      Luke Shumaker authored and Eli Schwartz's avatar Eli Schwartz committed
      It is a method of notifying text-editors that a file is in Bash syntax
      without giving it a propper shebang (which would be confusing, as it
      would suggest that the file should be executable), as well as working
      across virtually all text-editors (unlike "-*- Mode: Bash -*-" or
      whatever).
      97e17a59
    • Luke Shumaker's avatar
      Normalize to tab indent · 9672d6ec
      Luke Shumaker authored and Eli Schwartz's avatar Eli Schwartz committed
      9672d6ec
    • Luke Shumaker's avatar
      41333413
  13. 16 Mar, 2018 4 commits
    • Eli Schwartz's avatar
      fixup! Update messages to make fuller use of printf formatters · 5c867ea3
      Eli Schwartz authored
      pkgs should be passed as a single argument
      5c867ea3
    • Luke Shumaker's avatar
      test: Fixup glob matching · da49ea61
      Luke Shumaker authored and Eli Schwartz's avatar Eli Schwartz committed
       - ftpdir-cleanup.bats: Glob expansion does not occur in [[ -f ]] tests.
         The [[ ! -f .../${pkgname}-*${PKGEXT} ]] checks were checking that there
         were no files containing a literal '*' for that part of their name.
         Obviously, this isn't what was intended.
      
       - sourceballs.bats: [ -r ] checks explode if the glob returns >1 file.
         Avoid using them if the path being checked contains a glob.
      da49ea61
    • Luke Shumaker's avatar
      test: common.bash:__getCheckSum: Don't rely on IFS · 4ae3ea2f
      Luke Shumaker authored and Eli Schwartz's avatar Eli Schwartz committed
      I managed to stumble across a bug in BATS where the run() function
      screwed with the global IFS.  The bug has been fixed in git, but isn't
      in a release yet.
      
      https://github.com/sstephenson/bats/issues/89
      
      Anyway, this bug breaks __getCheckSum().  Fortunately, we have avoided
      tripping it so far because luck has it that we never call
      __getCheckSum() after run() in the same test.
      
      So, there's nothing actually broken here, but it makes me nervous.  So
      go ahead and modify __getCheckSum to not rely on IFS.
      
      And, while we're at it: declare the result variable and set it as
      separate commands.  Doing both in the same command masks the exit code
      of the subshell expansion.  We don't explicitly check the exit code,
      but BATS runs the test suite with `set -e`, so splitting it does mean
      that BATS will now detect errors from sha1sum.  We don't really expect
      that to happen, but if BATS will give us error checking on it for
      free, why not?
      4ae3ea2f
    • Luke Shumaker's avatar
      Update messages to make fuller use of printf formatters · 33aae318
      Luke Shumaker authored and Eli Schwartz's avatar Eli Schwartz committed
      These are things that were (IMO) missed in 5afac1ed.  I found them using:
      
          git grep -E '(plain|msg|msg2|warning|error|die) "[^"]*\$'
      
      I went a little above-and-beyond for escaping strings for the error
      messages in db-functions' arch_repo_add and arch_repo_remove.  The
      code should explain itself, but I wanted to point it out, as it's more than
      the usual "slap %s in there, and move the ${...} to the right".
      33aae318
  14. 26 Feb, 2018 1 commit
  15. 22 Feb, 2018 1 commit
    • Eli Schwartz's avatar
      Globally set $PKGEXT to a bash extended glob representing valid choices. · e53cad6e
      Eli Schwartz authored
      The current glob `*.pkg.tar.?z` is both less restrictive and more
      restrictive than makepkg, as it accepts any valid unicode character.
      
      To be more exact, it's almost completely orthogonal to the one in makepkg.
      
      makepkg only accepts .tar.gz, .tar.bz2, .tar.xz, .tar.lzo, .tar.lrz, and
      .tar.Z and most of those fail to match against a two-char compression type.
      
      dbscripts accepts .pkg.tar.💩z which incidentally is what I think of
      cherry-picking xz and gz as supported methods.
      
      Since this can be anything makepkg.conf accepts, it needs to be able to
      match all that, unless we decide to perform additional restrictions in
      which case we should still explicitly list each allowed extension. Using
      bash extended globbing allows us to do this relatively painlessly.
      
      Document the fact that this has *always* been some sort of glob, and
      update the two cases where this was (not!) being evaluated by bash
      [[ ... ]], to use a not-elegant-at-all proxy function is_globfile() to
      evaluate globs *before* testing if they exist.
      e53cad6e
  16. 20 Feb, 2018 5 commits
    • Eli Schwartz's avatar
      ftpdir-cleanup,sourceballs: replace external find command with bash globbing · c82f4372
      Eli Schwartz authored
      This fully removes the use of find from the codebase, leads to a
      micro-optimization in a couple cases, and ensures that $PKGEXT is
      consistently treated as a shell globbing character (which is important
      because it is used as one).
      
      Of the eight instances in these files:
      
      - One was unnecessary as `cat` can natively consume all files passed to
        it and no directory traversal was in use.
      
      - Two were unnecessary as they were hardcoded to read a single file....
      
      - Another four were only being used to strip leading directory paths,
        and can be replaced by globstar and ${filepath##*/}
      
      - The final two were checking the modification time of the files, and
        can be replaced with touch(1) and [[ -nt ]]. Although this introduces
        an additional temporary file, this is not such a big deal.
      c82f4372
    • Eli Schwartz's avatar
      db-update: replace external find command with bash globbing · 3e01ba9a
      Eli Schwartz authored
      Don't bother emitting errors. bash doesn't show globbing errors if it
      cannot read a directory to try globbing there. And the former code never
      aborted on errors anyway, as without `set -o pipefail` the sort command
      swallowed the return code.
      3e01ba9a
    • Luke Shumaker's avatar
      test: db-update: verify that PKGEXT(S) is treated as a glob · 8757c459
      Luke Shumaker authored and Eli Schwartz's avatar Eli Schwartz committed
      Commit b61a7148 introduced a regression that in a db-functions function
      called by db-update, config:PKGEXT is treated like a fixed string, instead
      of as a glob.  Because of inadequacy of the test-suite, this did not cause
      a test failure.
      
      https://lists.archlinux.org/pipermail/arch-projects/2018-February/004742.html
      
      The broken function checks if the repo already contains a package that
      matches the one being released.  This did not trigger a test-suite failure
      because right after the broken check there is another check that checks for
      the exact filename of the new package file.  In the test-suite, all
      package files have the same extension, so the latter check always "saved"
      us.
      
      So, modify a relevant test case (a test case that verifies that we can't
      release the same package twice) such that the second time we release the
      package, we set PKGEXT to something different (.gz, instead of the normal
      .xz).  This way, we can be sure that when db-update rejects the operation,
      it's because of the glob check, not the exact-filename check.
      
      This is made slightly tricky because the __buildPackage() test helper
      routine tries to cache builds, to speed up the tests.  Without
      modification, this means that it will just ignore the new PKGEXT.  So,
      modify it to only use the cache if it can successfully copy build artifacts
      out, in a way that respects PKGEXT.
      8757c459
    • Eli Schwartz's avatar
      Fix overloading PKGEXT to mean two things. · 07e6a91d
      Eli Schwartz authored
      PKGEXT is a makepkg variable referring to a fixed filename suffix, but
      we were also using it to mean a bash glob referring to candidate
      filenames. This is wrong, so rename it to PKGEXTS which is more
      descriptive of its purpose.
      
      Exclude the testsuite from this change, as the testsuite actually uses
      PKGEXT for its intended purpose. Fix the testsuite to consistently use
      PKGEXT, as it hardcoded the file extension in several cases, and pin
      its value to .pkg.tar.xz
      07e6a91d
    • Eli Schwartz's avatar
      Use even more bashisms. · b45650c3
      Eli Schwartz authored
      Catch some cases that were missed in the previous run.
      b45650c3