GNOME.org
 

[Notes] [Git][BuildGrid/buildgrid][laurence/update-policies] 5 commits: client/cas.py: Preserve file executability on download

Title: GitLab

Martin Blanchard pushed to branch laurence/update-policies at BuildGrid / buildgrid

Commits:

5 changed files:

Changes:

  • COMMITTERS.md
    1 
    +## COMMITTERS
    2 
    +
    3 
    +| Name | Email |
    4 
    +| -------- | -------- |
    5 
    +|  Carter Sande  | <carter.sande@duodecima.technology> |
    6 
    +|  Ed Baunton  | <edbaunton gmail com> |
    7 
    +|  Laurence Urhegyi  | <laurence urhegyi codethink co uk> |
    8 
    +|  Finn Ball  | <finn ball codethink co uk> |
    9 
    +|  Paul Sherwood  | <paul sherwood codethink co uk> |
    10 
    +|  James Ennis  | <james ennis codethink com> |
    11 
    +|  Jim MacArthur  | <jim macarthur codethink co uk> |
    12 
    +|  Juerg Billeter  | <juerg billeter codethink co uk> |
    13 
    +|  Martin Blanchard  | <martin blanchard codethink co uk> |
    14 
    +|  Marios Hadjimichael  | <mhadjimichae bloomberg net> |
    15 
    +|  Raoul Hidalgo Charman  | <raoul hidalgocharman codethink co uk> |
    16 
    +|  Rohit Kothur  |  <rkothur bloomberg net> |

  • CONTRIBUTING.rst
    ... ... @@ -32,40 +32,31 @@ side effects and quirks the feature may have introduced. More on this below in
    32 32
    33 33 
     .. _BuildGrid mailing list: https://lists.buildgrid.build/cgi-bin/mailman/listinfo/buildgrid
    34 34
    35 
    -
    36 35 
     .. _patch-submissions:
    37 36
    38 37 
     Patch submissions
    39 38 
     -----------------
    40 39
    41 
    -We are running `trunk based development`_. The idea behind this is that merge
    42 
    -requests to the trunk will be small and made often, thus making the review and
    43 
    -merge process as fast as possible. We do not want to end up with a huge backlog
    44 
    -of outstanding merge requests. If possible, it is preferred that merge requests
    45 
    -address specific points and clearly outline what problem they are solving.
    46 
    -
    47 
    -Branches must be submitted as merge requests (MR) on GitLab and should be
    48 
    -associated with an issue, whenever possible. If it's a small change, we'll
    49 
    -accept an MR without it being associated to an issue, but generally we prefer an
    50 
    -issue to be raised in advance. This is so that we can track the work that is
    40 
    +Branches must be submitted as merge requests (MR) on GitLab and should have a
    41 
    +corresponding issue raised in advance (whenever possible). If it's a small change,
    42 
    +an MR without it being associated to an issue is okay, but generally we prefer an
    43 
    +issue to be raised in advance so that we can track the work that is
    51 44 
     currently in progress on the project.
    52 45
    46 
    +When submitting a merge request, please obtain a review from another committer
    47 
    +who is familiar with the area of the code base which the branch effects. An
    48 
    +approval from another committer who is not the patch author will be needed
    49 
    +before any merge (we use Gitlab's 'approval' feature for this).
    50 
    +
    53 51 
     Below is a list of good patch submission good practice:
    54 52
    55 53 
     - Each commit should address a specific issue number in the commit message. This
    56 54 
       is really important for provenance reasons.
    57 
    -- Merge requests that are not yet ready for review must be prefixed with the
    58 
    -  ``WIP:`` identifier, but if we stick to trunk based development then the
    59 
    -  ``WIP:`` identifier will not stay around for very long on a merge request.
    60 
    -- When a merge request is ready for review, please find someone willing to do
    61 
    -  the review (ideally a maintainer) and assign them the MR, leaving a comment
    62 
    -  asking for their review.
    55 
    +- Merge requests that are not yet ready for review should be prefixed with the
    56 
    +  ``WIP:`` identifier.
    63 57 
     - Submitted branches should not contain a history of work done.
    64 58 
     - Unit tests should be a separate commit.
    65 59
    66 
    -.. _trunk based development: https://trunkbaseddevelopment.com
    67 
    -
    68 
    -
    69 60 
     Commit messages
    70 61 
     ~~~~~~~~~~~~~~~
    71 62
    ... ... @@ -89,6 +80,57 @@ For more tips, please read `The seven rules of a great Git commit message`_.
    89 80
    90 81 
     .. _The seven rules of a great Git commit message: https://chris.beams.io/posts/git-commit/#seven-rules
    91 82
    83 
    +.. _committer-access:
    84 
    +
    85 
    +Committer access
    86 
    +----------------
    87 
    +
    88 
    +Committers in the BuildGrid project are those folks to whom the right to
    89 
    +directly commit changes to our version controlled resources has been granted.
    90 
    +While every contribution is
    91 
    +valued regardless of its source, not every person who contributes code to the
    92 
    +project will earn commit access. The `COMMITTERS`_ file lists all committers.
    93 
    +
    94 
    +.. _COMMITTERS: https://gitlab.com/BuildGrid/buildgrid/blob/master/COMMITTERS.md
    95 
    +.. _Subversion: http://subversion.apache.org/docs/community-guide/roles.html#committers
    96 
    +
    97 
    +
    98 
    +How commit access is granted
    99 
    +~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    100 
    +
    101 
    +After someone has successfully contributed a few non-trivial patches, some full
    102 
    +committer, usually whoever has reviewed and applied the most patches from that
    103 
    +contributor, proposes them for commit access. This proposal is sent only to the
    104 
    +other full committers -- the ensuing discussion is private, so that everyone can
    105 
    +feel comfortable speaking their minds. Assuming there are no objections, the
    106 
    +contributor is granted commit access. The decision is made by consensus; there
    107 
    +are no formal rules governing the procedure, though generally if someone strongly
    108 
    +objects the access is not offered, or is offered on a provisional basis.
    109 
    +
    110 
    +This of course relies on contributors being responsive and showing willingness
    111 
    +to address any problems that may arise after landing patches. However, the primary
    112 
    +criterion for commit access is good judgment.
    113 
    +
    114 
    +You do not have to be a technical wizard, or demonstrate deep knowledge of the
    115 
    +entire codebase to become a committer. You just need to know what you don't
    116 
    +know. If your patches adhere to the guidelines in this file, adhere to all the usual
    117 
    +unquantifiable rules of coding (code should be readable, robust, maintainable, etc.),
    118 
    +and respect the Hippocratic Principle of "first, do no harm", then you will probably
    119 
    +get commit access pretty quickly. The size, complexity, and quantity of your patches
    120 
    +do not matter as much as the degree of care you show in avoiding bugs and minimizing
    121 
    +unnecessary impact on the rest of the code. Many full committers are people who have
    122 
    +not made major code contributions, but rather lots of small, clean fixes, each of
    123 
    +which was an unambiguous improvement to the code. (Of course, this does not mean the
    124 
    +project needs a bunch of very trivial patches whose only purpose is to gain commit
    125 
    +access; knowing what's worth a patch post and what's not is part of showing good
    126 
    +judgement.)
    127 
    +
    128 
    +When submitting a merge request, please obtain a review from another committer
    129 
    +who is familiar with the area of the code base which the branch effects. Asking on
    130 
    +slack is probably the best way to go about this. An approval from a committer
    131 
    +who is not the patch author will be needed before any merge (we use Gitlab's 'approval'
    132 
    +feature for this).
    133 
    +
    92 134
    93 135 
     .. _coding-style:
    94 136
    ... ... @@ -198,35 +240,6 @@ trunk.
    198 240
    199 241 
     .. _coverage report: https://buildgrid.gitlab.io/buildgrid/coverage/
    200 242
    201 
    -
    202 
    -.. _committer-access:
    203 
    -
    204 
    -Committer access
    205 
    -----------------
    206 
    -
    207 
    -We'll hand out commit access to anyone who has successfully landed a single
    208 
    -patch to the code base. Please request this via Slack or the mailing list.
    209 
    -
    210 
    -This of course relies on contributors being responsive and showing willingness
    211 
    -to address any problems that may arise after landing branches.
    212 
    -
    213 
    -When submitting a merge request, please obtain a review from another committer
    214 
    -who is familiar with the area of the code base which the branch effects. An
    215 
    -approval from another committer who is not the patch author will be needed
    216 
    -before any merge (we use gitlab's 'approval' feature for this).
    217 
    -
    218 
    -What we are expecting of committers here in general is basically to escalate the
    219 
    -review in cases of uncertainty.
    220 
    -
    221 
    -.. note::
    222 
    -
    223 
    -   We don't have any detailed policy for "bad actors", but will of course handle
    224 
    -   things on a case by case basis - commit access should not result in commit
    225 
    -   wars or be used as a tool to subvert the project when disagreements arise.
    226 
    -   Such incidents (if any) would surely lead to temporary suspension of commit
    227 
    -   rights.
    228 
    -
    229 
    -
    230 243 
     .. _gitlab-features:
    231 244
    232 245 
     GitLab features

  • MAINTAINERS deleted
    1 
    -Finn Ball
    2 
    -E-mail: finn ball codethink co uk
    3 
    -Userid: finnball

  • buildgrid/client/cas.py
    ... ... @@ -171,7 +171,7 @@ class Downloader:
    171 171
    172 172 
             return messages
    173 173
    174 
    -    def download_file(self, digest, file_path, queue=True):
    174 
    +    def download_file(self, digest, file_path, is_executable=False, queue=True):
    175 175 
             """Retrieves a file from the remote CAS server.
    176 176
    177 177 
             If queuing is allowed (`queue=True`), the download request **may** be
    ... ... @@ -181,6 +181,7 @@ class Downloader:
    181 181 
             Args:
    182 182 
                 digest (:obj:`Digest`): the file's digest to fetch.
    183 183 
                 file_path (str): absolute or relative path to the local file to write.
    184 
    +            is_executable (bool): whether the file is executable or not.
    184 185 
                 queue (bool, optional): whether or not the download request may be
    185 186 
                     queued and submitted as part of a batch upload request. Defaults
    186 187 
                     to True.
    ... ... @@ -193,9 +194,9 @@ class Downloader:
    193 194 
                 file_path = os.path.abspath(file_path)
    194 195
    195 196 
             if not queue or digest.size_bytes > FILE_SIZE_THRESHOLD:
    196 
    -            self._fetch_file(digest, file_path)
    197 
    +            self._fetch_file(digest, file_path, is_executable=is_executable)
    197 198 
             else:
    198 
    -            self._queue_file(digest, file_path)
    199 
    +            self._queue_file(digest, file_path, is_executable=is_executable)
    199 200
    200 201 
         def download_directory(self, digest, directory_path):
    201 202 
             """Retrieves a :obj:`Directory` from the remote CAS server.
    ... ... @@ -311,7 +312,7 @@ class Downloader:
    311 312
    312 313 
             return read_blobs
    313 314
    314 
    -    def _fetch_file(self, digest, file_path):
    315 
    +    def _fetch_file(self, digest, file_path, is_executable=False):
    315 316 
             """Fetches a file using ByteStream.Read()"""
    316 317 
             if self.instance_name:
    317 318 
                 resource_name = '/'.join([self.instance_name, 'blobs',
    ... ... @@ -332,7 +333,10 @@ class Downloader:
    332 333
    333 334 
                 assert byte_file.tell() == digest.size_bytes
    334 335
    335 
    -    def _queue_file(self, digest, file_path):
    336 
    +        if is_executable:
    337 
    +            os.chmod(file_path, 0o755)  # rwxr-xr-x / 755
    338 
    +
    339 
    +    def _queue_file(self, digest, file_path, is_executable=False):
    336 340 
             """Queues a file for later batch download"""
    337 341 
             if self.__file_request_size + digest.ByteSize() > MAX_REQUEST_SIZE:
    338 342 
                 self.flush()
    ... ... @@ -341,22 +345,25 @@ class Downloader:
    341 345 
             elif self.__file_request_count >= MAX_REQUEST_COUNT:
    342 346 
                 self.flush()
    343 347
    344 
    -        self.__file_requests[digest.hash] = (digest, file_path)
    348 
    +        self.__file_requests[digest.hash] = (digest, file_path, is_executable)
    345 349 
             self.__file_request_count += 1
    346 350 
             self.__file_request_size += digest.ByteSize()
    347 351 
             self.__file_response_size += digest.size_bytes
    348 352
    349 353 
         def _fetch_file_batch(self, batch):
    350 354 
             """Sends queued data using ContentAddressableStorage.BatchReadBlobs()"""
    351 
    -        batch_digests = [digest for digest, _ in batch.values()]
    355 
    +        batch_digests = [digest for digest, _, _ in batch.values()]
    352 356 
             batch_blobs = self._fetch_blob_batch(batch_digests)
    353 357
    354 
    -        for (_, file_path), file_blob in zip(batch.values(), batch_blobs):
    358 
    +        for (_, file_path, is_executable), file_blob in zip(batch.values(), batch_blobs):
    355 359 
                 os.makedirs(os.path.dirname(file_path), exist_ok=True)
    356 360
    357 361 
                 with open(file_path, 'wb') as byte_file:
    358 362 
                     byte_file.write(file_blob)
    359 363
    364 
    +            if is_executable:
    365 
    +                os.chmod(file_path, 0o755)  # rwxr-xr-x / 755
    366 
    +
    360 367 
         def _fetch_directory(self, digest, directory_path):
    361 368 
             """Fetches a file using ByteStream.GetTree()"""
    362 369 
             # Better fail early if the local root path cannot be created:
    ... ... @@ -414,7 +421,7 @@ class Downloader:
    414 421 
             for file_node in root_directory.files:
    415 422 
                 file_path = os.path.join(root_path, file_node.name)
    416 423
    417 
    -            self._queue_file(file_node.digest, file_path)
    424 
    +            self._queue_file(file_node.digest, file_path, is_executable=file_node.is_executable)
    418 425
    419 426 
             for directory_node in root_directory.directories:
    420 427 
                 directory_path = os.path.join(root_path, directory_node.name)

  • tests/cas/data/hello/hello.sh
    1 
    +#!/bin/bash
    2 
    +
    3 
    +echo "Hello, World!"

    • —
    View it on GitLab.
    You're receiving this email because of your account on gitlab.com. If you'd like to receive fewer emails, you can adjust your notification settings.

    [Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]