Merge pull request #552 from jkloetzke/url-permissions

Fix a couple of URL SCM issues

doc/manual/configuration.rst

@@ -880,6 +880,10 @@ be given as IfExpression (see :ref:`configuration-principle-booleans`). By
 default the SCMs check out to the root of the workspace. You may specify any
 relative path in ``dir`` to checkout to this directory.
 
+.. hint::
+   The defaults of all attributes marked by an asterisk (\*) can be changed by
+   :ref:`configuration-config-scmDefaults` in the user configuration.
+
 Special care must be taken if SCMs are nested, that is the ``dir`` attribute of
 one SCM is a subdirectory of another. Bob requires that the SCM with the upper
 directory has to be in the list before the SCMs that are checked out into
@@ -900,52 +904,53 @@ will only be considered if the condition passes.
 
 Currently the following ``scm`` values are supported:
 
-====== ============================ =======================================================================================
-scm    Description                  Additional attributes
-====== ============================ =======================================================================================
-cvs    CVS repository               | ``cvsroot``: repository location ("``:ext:...``", path name, etc.)
-                                    | ``module``: module name
-                                    | ``rev``: revision, branch, or tag name (optional)
-git    `Git`_ project               | ``url``: URL of remote repository
-                                    | ``branch`` (\*): Branch to check out (optional, default: master)
-                                    | ``tag``: Checkout this tag (optional, overrides branch attribute)
-                                    | ``commit``: SHA1 commit Id to check out (optional, overrides branch or tag attribute)
-                                    | ``rev``: Canonical git-rev-parse revision specification (optional, see below)
-                                    | ``remote-*``: additional remote repositories (optional, see below)
-                                    | ``sslVerify`` (\*): Whether to verify the SSL certificate when fetching (optional)
-                                    | ``shallow`` (\*): Number of commits or cutoff date that should be fetched (optional)
-                                    | ``singleBranch`` (\*): Fetch only single branch instead of all (optional)
-                                    | ``submodules`` (\*): Whether to clone all / a subset of submodules. (optional)
-                                    | ``recurseSubmodules`` (\*): Recusively clone submodules (optional, defaults to false)
-                                    | ``shallowSubmodules`` (\*): Clone submodules shallowly (optional, defaults to true)
-                                    | ``references`` (\*): Git reference. A local reference repo to be used as
-                                    |       alternate (see man git-clone).
-                                    |       A list of strings or a dictionaries with
-                                    |        ``url``: (optional, Regex-String, default: ``.*``). The matching part
-                                    |           of the remote URL is replaced by
-                                    |        ``repo``: (String) local storage path.
-                                    |        ``optional``: (Boolean, default True). Marks the reference as
-                                    |           optional if true. Otherwise a error is raised if the
-                                    |           local reference repo didn't exitst.
-                                    |   Note: ``references`` are not used for submodules.
-                                    | ``retries`` (\*): Number of retries before the checkout is set to failed.
-                                    | ``disassociate``: (Boolean, default false). Diasassociate the reference.
-import Import directory from        | ``url``: Directory path relative to project root.
-       project                      | ``prune`` (\*): Delete destination directory before importing files.
-svn    `Svn`_ repository            | ``url``: URL of SVN module
-                                    | ``revision``: Optional revision number (optional)
-                                    | ``sslVerify`` (\*): Whether to verify the SSL certificate when fetching (optional)
-url    While not a real SCM it      | ``url``: File that should be downloaded
-       allows to download (and      | ``digestSHA1``: Expected SHA1 digest of the file (optional)
-       extract) files/archives.     | ``digestSHA256``: Expected SHA256 digest of the file (optional)
-                                    | ``digestSHA512``: Expected SHA512 digest of the file (optional)
-                                    | ``extract`` (\*): Extract directive (optional, default: auto)
-                                    | ``fileName`` (\*): Local file name (optional, default: url file name)
-                                    | ``sslVerify`` (\*): Whether to verify the SSL certificate when fetching (optional)
-                                    | ``stripComponents`` (\*): Number of leading components stripped from file name
-                                    |                           (optional, tar files only)
-                                    | ``retries`` (\*): Number of retries before the checkout is set to failed.
-====== ============================ =======================================================================================
+====== =======================================================================================
+scm    Additional attributes
+====== =======================================================================================
+cvs    | ``cvsroot``: repository location ("``:ext:...``", path name, etc.)
+       | ``module``: module name
+       | ``rev``: revision, branch, or tag name (optional)
+git    | ``url``: URL of remote repository
+       | ``branch`` (\*): Branch to check out (optional, default: master)
+       | ``tag``: Checkout this tag (optional, overrides branch attribute)
+       | ``commit``: SHA1 commit Id to check out (optional, overrides branch or tag attribute)
+       | ``rev``: Canonical git-rev-parse revision specification (optional, see below)
+       | ``remote-*``: additional remote repositories (optional, see below)
+       | ``sslVerify`` (\*): Whether to verify the SSL certificate when fetching (optional)
+       | ``shallow`` (\*): Number of commits or cutoff date that should be fetched (optional)
+       | ``singleBranch`` (\*): Fetch only single branch instead of all (optional)
+       | ``submodules`` (\*): Whether to clone all / a subset of submodules. (optional)
+       | ``recurseSubmodules`` (\*): Recusively clone submodules (optional, defaults to false)
+       | ``shallowSubmodules`` (\*): Clone submodules shallowly (optional, defaults to true)
+       | ``references`` (\*): Git reference. A local reference repo to be used as
+       |       alternate (see man git-clone).
+       |       A list of strings or a dictionaries with
+       |        ``url``: (optional, Regex-String, default: ``.*``). The matching part
+       |           of the remote URL is replaced by
+       |        ``repo``: (String) local storage path.
+       |        ``optional``: (Boolean, default True). Marks the reference as
+       |           optional if true. Otherwise a error is raised if the
+       |           local reference repo didn't exitst.
+       |   Note: ``references`` are not used for submodules.
+       | ``retries`` (\*): Number of retries before the checkout is set to failed.
+       | ``disassociate``: (Boolean, default false). Diasassociate the reference.
+import | ``url``: Directory path relative to project root.
+       | ``prune`` (\*): Delete destination directory before importing files.
+svn    | ``url``: URL of SVN module
+       | ``revision``: Optional revision number (optional)
+       | ``sslVerify`` (\*): Whether to verify the SSL certificate when fetching (optional)
+url    | ``url``: File that should be downloaded
+       | ``digestSHA1``: Expected SHA1 digest of the file (optional)
+       | ``digestSHA256``: Expected SHA256 digest of the file (optional)
+       | ``digestSHA512``: Expected SHA512 digest of the file (optional)
+       | ``extract`` (\*): Extract directive (optional, default: auto)
+       | ``fileName`` (\*): Local file name (optional, default: url file name)
+       | ``fileMode`` (\*): File mode (optional, default depends on :ref:`policies-defaultFileMode` policy)
+       | ``sslVerify`` (\*): Whether to verify the SSL certificate when fetching (optional)
+       | ``stripComponents`` (\*): Number of leading components stripped from file name
+       |                           (optional, tar files only)
+       | ``retries`` (\*): Number of retries before the checkout is set to failed.
+====== =======================================================================================
 
 The following synthetic attributes exist. They are generated internally
 and cannot be set in the recipe. They are intended to be matched in queries
@@ -1089,7 +1094,7 @@ import
       otherwise.
 
 svn
-   The Svn SCM, like git, requires the ``url`` attribute too. If you specify a
+   The `Svn`_ SCM, like git, requires the ``url`` attribute too. If you specify a
    numeric ``revision`` Bob considers the SCM as deterministic.
 
 url
@@ -1118,6 +1123,13 @@ url
        possible to fetch multiple files in the same directory. This is done to
        separate possibly extracted files safely from other checkouts.
 
+   The file mode of the downloaded or copied file can be set with the
+   ``fileMode`` attribute. Two formats are supported: bit masks (as
+   octal number) or a symbolic string mode.  Both formats follow the ``chmod``
+   command syntax. Examples: ``0764``, ``"u=rwx,g=rw,o=r"``. The ``fileMode``
+   defaults to ``0600``/``"u=rw"`` unless the :ref:`policies-defaultFileMode`
+   policy is configured for the old behaviour.
+
 .. _configuration-recipes-checkoutUpdateIf:
 
 checkoutUpdateIf

doc/manual/policies.rst

@@ -592,3 +592,32 @@ Old behavior
 New behavior
    Changes to the ``dir`` attribute of an ``import`` SCM behave the same as for
    any other SCM.
+
+.. _policies-defaultFileMode:
+
+defaultFileMode
+~~~~~~~~~~~~~~~
+
+Introduced in: 0.24
+
+The URL SCM applies a file mode of ``0600`` (user read/write only) to all files
+that are fetched via HTTP(S) or FTP. For locally copied files (``file://``
+URLs or bare file names) though, the file mode of the source file is retained.
+This can lead to unstable builds, e.g. if the file source is overridden by an
+``scmOverrides`` entry or if a mirror is used. Bob also did not consider the
+file mode to be part of the :term:`Variant-Id` even though it influences the
+build result.
+
+Starting with Bob 0.24, the file mode can be specified by the ``fileMode``
+attribute. It is then also part of the :term:`Variant-Id` and will trigger
+rebuilds if changed. This policy governs the default of the ``fileMode``
+attribute to enable a consistent behavior, regardless of the URL schema.
+
+Old behavior
+    The mode of files coped from ``file://`` URLs or bare file names is
+    retained from the source unless the ``fileMode`` attribute overrides
+    it explicitly.
+
+New behavior
+    The ``fileMode`` attribute is default initialized to ``0600``. All files
+    will get the same mode, regardless of the URL schema.

pym/bob/builder.py

@@ -10,6 +10,7 @@
 from .input import RecipeSet
 from .invoker import Invoker, InvocationMode
 from .languages import StepSpec
+from .scm import getScm
 from .state import BobState
 from .stringparser import Env
 from .tty import log, stepMessage, stepAction, stepExec, setProgress, ttyReinit, \
@@ -1183,7 +1184,7 @@ async def _cookCheckoutStep(self, checkoutStep, depth):
                     elif scmDigest != checkoutState.get(scmDir, (None, None))[0]:
                         canSwitch = (scmDir in scmMap) and scmDigest and \
                                      scmSpec is not None and \
-                                     scmMap[scmDir].canSwitch(scmSpec) and \
+                                     scmMap[scmDir].canSwitch(getScm(scmSpec)) and \
                                      os.path.exists(scmPath)
                         didSwitch = False
                         if canSwitch:

pym/bob/cmds/show.py

@@ -46,7 +46,7 @@ def dumpPackage(package):
         doc["meta"]["checkoutVariantId"] = asHexStr(checkoutStep.getVariantId())
         doc["checkoutDeterministic"] = checkoutStep.isDeterministic()
         doc["checkoutSCM"] = [
-            { k:v for k,v in s.getProperties(False).items() if not k.startswith("__") }
+            { k:v for k,v in s.getProperties(False, True).items() if not k.startswith("__") }
             for s in checkoutStep.getScmList()
         ]
         doc["checkoutAssert"] = [

pym/bob/input.py

@@ -3161,6 +3161,11 @@ def __init__(self):
                 InfoOnce("fixImportScmVariant policy not set. Recipe variant calculation w/ import SCM is boguous.",
                     help="See http://bob-build-tool.readthedocs.io/en/latest/manual/policies.html#fiximportscmvariant for more information.")
             ),
+            "defaultFileMode": (
+                "0.24",
+                InfoOnce("defaultFileMode policy not set. File mode of URL SCMs not set for locally copied files.",
+                    help="See http://bob-build-tool.readthedocs.io/en/latest/manual/policies.html#defaultfilemode for more information.")
+            ),
         }
         self.__buildHooks = {}
         self.__sandboxOpts = {}

pym/bob/intermediate.py

@@ -565,6 +565,7 @@ def fromRecipeSet(cls, recipeSet):
             'sandboxFingerprints' : recipeSet.getPolicy('sandboxFingerprints'),
             'gitCommitOnBranch' : recipeSet.getPolicy('gitCommitOnBranch'),
             'fixImportScmVariant' : recipeSet.getPolicy('fixImportScmVariant'),
+            'defaultFileMode' : recipeSet.getPolicy('defaultFileMode'),
         }
         self.__data['archiveSpec'] = recipeSet.archiveSpec()
         self.__data['envWhiteList'] = sorted(recipeSet.envWhiteList())

pym/bob/invoker.py

@@ -486,7 +486,7 @@ async def executeScmSwitch(self, scm, oldSpec):
         try:
             self.__openLog("SCM inline switch")
             try:
-                await scm.switch(self, oldSpec)
+                await scm.switch(self, getScm(oldSpec))
             except CmdFailedError as e:
                 self.error(scm.getSource(), "failed")
                 self.error(e.what)

pym/bob/scm/__init__.py

@@ -61,6 +61,7 @@ def getScm(spec, overrides=[], recipeSet=None):
         return UrlScm(spec, overrides, recipeSet and recipeSet.getPolicy('tidyUrlScm'),
             recipeSet and recipeSet.getPolicy('scmIgnoreUser'),
             recipeSet and recipeSet.getPreMirrors(),
-            recipeSet and recipeSet.getFallbackMirrors())
+            recipeSet and recipeSet.getFallbackMirrors(),
+            recipeSet and recipeSet.getPolicy('defaultFileMode'))
     else:
         raise ParseError("Unknown SCM '{}'".format(scm))

pym/bob/scm/cvs.py

@@ -35,7 +35,7 @@ def __init__(self, spec, overrides=[]):
         self.__rev = spec.get("rev")
         self.__dir = spec.get("dir", ".")
 
-    def getProperties(self, isJenkins):
+    def getProperties(self, isJenkins, pretty=False):
         ret = super().getProperties(isJenkins)
         ret.update({
             'scm' : 'cvs',

pym/bob/scm/git.py

@@ -133,7 +133,7 @@ def __resolveReferences(self, alt):
             self.__resolvedReferences = [ ref for ref in [__resolveReferences(self, a) for a in
                 self.__references]]
 
-    def getProperties(self, isJenkins):
+    def getProperties(self, isJenkins, pretty=False):
         properties = super().getProperties(isJenkins)
         properties.update({
             'scm' : 'git',
@@ -503,26 +503,28 @@ async def __updateSubmodulesPost(self, invoker, oldState, base = "."):
                 await self.__updateSubmodulesPost(invoker, subMods[p],
                                                   os.path.join(base, p))
 
-    def canSwitch(self, oldSpec):
-        diff = self._diffSpec(oldSpec)
+    def canSwitch(self, oldScm):
+        diff = self._diffSpec(oldScm)
+        if "scm" in diff:
+            return False
 
         # Filter irrelevant properties
         diff -= {"sslVerify", 'singleBranch', 'shallow',
                 'shallowSubmodules', "useBranchAndCommit",
-                "references", "dissociate"}
+                "references", "dissociate", "retries"}
 
         diff = set(prop for prop in diff if not prop.startswith("remote-"))
 
         # Enabling "submodules" and/or "recurseSubmodules" is ok. The
         # additional content will be checked out in invoke().
-        if not oldSpec.get("submodules", False) and self.__submodules:
+        if not oldScm.__submodules and self.__submodules:
             diff.discard("submodules")
-        if not oldSpec.get("recursiveSubmodules", False) and self.__recurseSubmodules:
-            diff.discard("recursiveSubmodules")
+        if not oldScm.__recurseSubmodules and self.__recurseSubmodules:
+            diff.discard("recurseSubmodules")
 
-        # Without submodules the recursiveSubmodules property is irrelevant
+        # Without submodules the recurseSubmodules property is irrelevant
         if not self.__submodules:
-            diff.discard("recursiveSubmodules")
+            diff.discard("recurseSubmodules")
 
         # For the rest we can try a inline switch. Git does not handle
         # vanishing submodules well and neither do we. So if submodules are
@@ -535,7 +537,7 @@ def canSwitch(self, oldSpec):
             return False
         return True
 
-    async def switch(self, invoker, oldSpec):
+    async def switch(self, invoker, oldScm):
         # Special handling for repositories that are in a detached HEAD state.
         # While it is technically ok to make an inline switch, the user can
         # only recover his old commit(s) from the reflog. This is confusing and
@@ -546,7 +548,8 @@ async def switch(self, invoker, oldSpec):
         if detached:
             # The only exception to the above rule is when a tag or commit
             # was checked out and the repo is still at this commit.
-            _, oldTag, oldCommit = getBranchTagCommit(oldSpec)
+            oldTag = oldScm.__tag
+            oldCommit = oldScm.__commit
             if oldCommit:
                 pass # just compare this commit
             elif oldTag:

pym/bob/scm/imp.py

@@ -136,7 +136,7 @@ def __init__(self, spec, overrides=[], pruneDefault=None, fixDigestBug=False, pr
         self.__projectRoot = spec.get("__projectRoot", projectRoot)
         self.__fixDigestBug = fixDigestBug
 
-    def getProperties(self, isJenkins):
+    def getProperties(self, isJenkins, pretty=False):
         ret = super().getProperties(isJenkins)
         ret.update({
             'scm' : 'import',