Merge ~racb/git-ubuntu:prepare-upload-fetch into git-ubuntu:main
- Git
- lp:~racb/git-ubuntu
- prepare-upload-fetch
- Merge into main
Status: | Merged | ||||||||
---|---|---|---|---|---|---|---|---|---|
Merged at revision: | 4fd07c835ab4f3f25d1b9296000986b88886b483 | ||||||||
Proposed branch: | ~racb/git-ubuntu:prepare-upload-fetch | ||||||||
Merge into: | git-ubuntu:main | ||||||||
Diff against target: |
1106 lines (+709/-141) 3 files modified
gitubuntu/prepare_upload.py (+401/-91) gitubuntu/prepare_upload_test.py (+299/-43) sandbox/gu-build (+9/-7) |
||||||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Server Team CI bot | continuous-integration | Approve | |
Steve Langasek | Approve | ||
git-ubuntu developers | Pending | ||
Review via email: mp+445861@code.launchpad.net |
Commit message
Make Jenkins happy
Description of the change
Robie Basak (racb) wrote : | # |
Server Team CI bot (server-team-bot) wrote : | # |
FAILED: Continuous integration, rev:a384e90b271
https:/
Executed test runs:
SUCCESS: VM Setup
SUCCESS: Build
SUCCESS: VM Reset
FAILED: Unit Tests
Click here to trigger a rebuild:
https:/
Server Team CI bot (server-team-bot) wrote : | # |
PASSED: Continuous integration, rev:9cc9c226113
https:/
Executed test runs:
SUCCESS: VM Setup
SUCCESS: Build
SUCCESS: VM Reset
SUCCESS: Unit Tests
IN_PROGRESS: Declarative: Post Actions
Click here to trigger a rebuild:
https:/
Server Team CI bot (server-team-bot) wrote : | # |
PASSED: Continuous integration, rev:ebf85d59b9c
https:/
Executed test runs:
SUCCESS: VM Setup
SUCCESS: Build
SUCCESS: VM Reset
SUCCESS: Unit Tests
IN_PROGRESS: Declarative: Post Actions
Click here to trigger a rebuild:
https:/
Server Team CI bot (server-team-bot) wrote : | # |
PASSED: Continuous integration, rev:c52820bbfef
https:/
Executed test runs:
SUCCESS: VM Setup
SUCCESS: Build
SUCCESS: VM Reset
SUCCESS: Unit Tests
IN_PROGRESS: Declarative: Post Actions
Click here to trigger a rebuild:
https:/
Server Team CI bot (server-team-bot) wrote : | # |
PASSED: Continuous integration, rev:c52820bbfef
https:/
Executed test runs:
SUCCESS: VM Setup
SUCCESS: Build
SUCCESS: VM Reset
SUCCESS: Unit Tests
IN_PROGRESS: Declarative: Post Actions
Click here to trigger a rebuild:
https:/
Server Team CI bot (server-team-bot) wrote : | # |
PASSED: Continuous integration, rev:2b275773407
https:/
Executed test runs:
SUCCESS: VM Setup
SUCCESS: Build
SUCCESS: VM Reset
SUCCESS: Unit Tests
IN_PROGRESS: Declarative: Post Actions
Click here to trigger a rebuild:
https:/
Robie Basak (racb) wrote : | # |
This is ready for review I think. Steve, could you review please, as it's mainly your request and also there are implications for gu-build?
Steve Langasek (vorlon) wrote : | # |
LGTM; one comment.
Robie Basak (racb) wrote : | # |
On Tue, Jul 18, 2023 at 12:10:48AM -0000, Steve Langasek wrote:
> The 'fixme' above this is addressed now by prepare_
We can, though I see _add_subparser_
implementation detail that I don't think would develop into an API for
other subcommands. In contrast the functions further up the call stack
could be more stable though we don't really know that either.
Given that we don't really have any API stability guarantees and also
expect gu-build to break if the CLI arguments were to change (since we
then parse the ArgumentParser.
so that would break) I'm not sure this change would gain anything. But
nor would it lose anything. So whatever you prefer for gu-build I guess?
Steve Langasek (vorlon) wrote : | # |
My preference is to roll with the possibility of future API breakage rather than duplicating the code here.
Robie Basak (racb) wrote : | # |
OK. Done.
Server Team CI bot (server-team-bot) wrote : | # |
PASSED: Continuous integration, rev:4fd07c835ab
https:/
Executed test runs:
SUCCESS: VM Setup
SUCCESS: Build
SUCCESS: VM Reset
SUCCESS: Unit Tests
IN_PROGRESS: Declarative: Post Actions
Click here to trigger a rebuild:
https:/
Preview Diff
1 | diff --git a/gitubuntu/prepare_upload.py b/gitubuntu/prepare_upload.py |
2 | index 93edd85..6b1caa5 100644 |
3 | --- a/gitubuntu/prepare_upload.py |
4 | +++ b/gitubuntu/prepare_upload.py |
5 | @@ -1,9 +1,9 @@ |
6 | """Prepare an upload by pushing rich history |
7 | |
8 | This subcommand provides two further subcommands: "args" and "mangle". Both |
9 | -subcommands first "git push" so that the rich history is made available to the |
10 | -importer when it later imports your upload. They then diverge in how they |
11 | -provide a pointer to that rich history. |
12 | +subcommands first "git fetch" and then "git push" if required so that the rich |
13 | +history is made available to the importer when it later imports your upload. |
14 | +They then diverge in how they provide a pointer to that rich history. |
15 | |
16 | The "args" subcommand prints command line arguments suitable to provide to |
17 | another command that will generate the changes file. It defaults to printing |
18 | @@ -14,6 +14,7 @@ fields. It requires a single positional argument pointing to the changes file |
19 | to mangle. |
20 | """ |
21 | import argparse |
22 | +import collections |
23 | import os |
24 | import re |
25 | import sys |
26 | @@ -54,9 +55,28 @@ URL_REWRITE = ( |
27 | ) |
28 | |
29 | |
30 | -def parse_args(subparsers=None, base_subparsers=None): |
31 | +class ForcePushRequired(RuntimeError): pass |
32 | + |
33 | + |
34 | +def _add_subparser_arguments(parser): |
35 | + """Load standard prepare-upload arguments into the given parser |
36 | + |
37 | + This is so that we can do this for the general prepare-upload subparser, as |
38 | + well as the specific args and mangle subparsers, so the arguments work |
39 | + regardless of whether they are placed before or after the final args/mangle |
40 | + subcommand. See: https://stackoverflow.com/a/46962350/478206 |
41 | + |
42 | + :param argparse.ArgumentParser parser: the parser to which toadd the |
43 | + arguments. |
44 | + """ |
45 | + parser.add_argument('--remote', type=str, help="Remote to push to") |
46 | + parser.add_argument('--branch', type=str, help="Branch to push") |
47 | + parser.add_argument('--force-push', action='store_true') |
48 | + |
49 | + |
50 | +def parse_args(subparsers=None, base_subparsers=None): # pragma: no cover |
51 | kwargs = dict( |
52 | - description="Push a branch for subsequent upload with dput", |
53 | + description="Push branch and provide history-enriched headers for dput", |
54 | formatter_class=argparse.ArgumentDefaultsHelpFormatter, |
55 | ) |
56 | if base_subparsers: |
57 | @@ -65,85 +85,201 @@ def parse_args(subparsers=None, base_subparsers=None): |
58 | parser = subparsers.add_parser('prepare-upload', **kwargs) |
59 | else: |
60 | parser = argparse.ArgumentParser(**kwargs) |
61 | - parser.add_argument('--remote', type=str, help="Remote to push to") |
62 | - parser.add_argument('--branch', type=str, help="Branch to push") |
63 | + |
64 | + _add_subparser_arguments(parser) |
65 | level2_subparsers = parser.add_subparsers(required=True) |
66 | - printargs_subparser = level2_subparsers.add_parser('args') |
67 | + printargs_subparser = level2_subparsers.add_parser( |
68 | + 'args', |
69 | + description="Push branch and print history-enriched header arguments", |
70 | + ) |
71 | printargs_subparser.set_defaults(func=cli_printargs) |
72 | + _add_subparser_arguments(printargs_subparser) |
73 | printargs_subparser.add_argument( |
74 | '--output-format', |
75 | choices=OUTPUT_FORMATS.keys(), |
76 | default='dpkg-buildpackage', |
77 | help="Output format", |
78 | ) |
79 | - manglechanges_subparser = level2_subparsers.add_parser('mangle') |
80 | + manglechanges_subparser = level2_subparsers.add_parser( |
81 | + 'mangle', |
82 | + description="Push branch and adjust history-enriched headers in changes file", |
83 | + ) |
84 | manglechanges_subparser.set_defaults(func=cli_manglechanges) |
85 | + _add_subparser_arguments(manglechanges_subparser) |
86 | manglechanges_subparser.add_argument('changes_file_path') |
87 | if not subparsers: |
88 | return parser.parse_args() |
89 | return "prepare-upload - %s" % kwargs['description'] |
90 | |
91 | |
92 | +class Parameters(collections.namedtuple('Parameters', [ |
93 | + 'vcs_git', |
94 | + 'vcs_git_ref', |
95 | + 'vcs_git_commit', |
96 | + 'commit', |
97 | + 'remote_name', |
98 | + 'local_ref_name', |
99 | + 'local_tracking_ref_name', |
100 | +])): |
101 | + """The parameters associated with a prepare-upload request |
102 | + |
103 | + When preparing an upload, we need to know what the final Vcs-Git* headers |
104 | + in the changes file will be, as well as various other parameters such as |
105 | + the name of the configured remote that corresponds to these, the pygit2 |
106 | + Commit object and so forth. These are determined once and then stored in an |
107 | + instance of this class, with the individual parameters specified here. |
108 | + |
109 | + :ivar str vcs_git: the contents of the eventual Vcs-Git header |
110 | + :ivar str vcs_git_ref: the contents of the eventual Vcs-Git-Ref header |
111 | + :ivar str vcs_git_commit: the contents of the eventual Vcs-Git-Commit |
112 | + header |
113 | + :ivar pygit2.Commit commit: the local commit being supplied as rich history |
114 | + :ivar str remote_name: the name of the configured remote where the rich |
115 | + history will be staged |
116 | + :ivar str local_ref_name: the ref name of the local branch that contains |
117 | + the rich history, such as "refs/heads/feature". |
118 | + :ivar str local_tracking_ref_name: the ref name of the local remote |
119 | + tracking branch that tracks the remote branch where the rich history |
120 | + will be staged, such as "refs/remotes/dave/feature". |
121 | + """ |
122 | + pass |
123 | + |
124 | + @property |
125 | + def changes_file_headers(self): |
126 | + """The changes file headers as a dict keyed on header name |
127 | + |
128 | + The header names are converted from the snake_case attributes to |
129 | + deb822-case used in changes files. |
130 | + """ |
131 | + return { |
132 | + k: getattr(self, k.lower().replace('-', '_')) |
133 | + for k in ['Vcs-Git', 'Vcs-Git-Ref', 'Vcs-Git-Commit'] |
134 | + } |
135 | + |
136 | + @classmethod |
137 | + def from_repo(cls, repo, remote_name, branch_name): |
138 | + """Instantiate Parameters from user-provided string args and defaults |
139 | + |
140 | + This method encapsulates the heuristics used to determine what to do |
141 | + based the state of the repository and anything explicitly specified by |
142 | + the user. |
143 | + |
144 | + :param GitUbuntuRepository repo: the repository to consider. |
145 | + :param str remote_name: the remote name explicitly requested by the |
146 | + user, or None. |
147 | + :param str branch_name: the branch name explicitly requested by the |
148 | + user, or None. |
149 | + :rtype: Parameters |
150 | + :returns: a populated instance of this Parameters class. |
151 | + """ |
152 | + if not remote_name: |
153 | + remote_name = repo.lp_user |
154 | + |
155 | + if branch_name: |
156 | + # XXX to identify local_tracking_ref_name, this much be a branch, not just any ref-ish |
157 | + commit, ref = repo.raw_repo.resolve_refish(branch_name) |
158 | + # ref might be None if the given branch parameter isn't actually a |
159 | + # branch (eg. is a commit) |
160 | + assert ref |
161 | + else: |
162 | + assert not repo.raw_repo.head_is_detached |
163 | + ref = repo.raw_repo.head |
164 | + assert ref.name.startswith('refs/heads/') |
165 | + branch_name = ref.shorthand |
166 | + commit = ref.peel(pygit2.Commit) |
167 | + |
168 | + pygit2_remote = repo.raw_repo.remotes[remote_name] |
169 | + |
170 | + # Rewrite rule for VCS URLs. See LP: #1942985 |
171 | + vcs_git = URL_REWRITE[0].sub(URL_REWRITE[1], pygit2_remote.url) |
172 | + |
173 | + vcs_git_ref = ref.name |
174 | + vcs_git_commit = str(commit.id) |
175 | + |
176 | + # Pre-empt any failure with later input validation of the headers we |
177 | + # will generate after we push. If these assertions fail, then the later |
178 | + # import will refuse to accept this rich history, so better that we |
179 | + # fail now so the uploader knows there's some kind of problem, rather |
180 | + # than later when the importer will silently throw away the rich |
181 | + # history that the uploader pushed. |
182 | + # |
183 | + # However, we don't know of any actual case when this might happen, so |
184 | + # these are assertions rather than fully UX-compliant error paths. |
185 | + assert gitubuntu.importer.VCS_GIT_URL_VALIDATION.fullmatch(vcs_git) |
186 | + assert vcs_git.startswith( |
187 | + gitubuntu.importer.LAUNCHPAD_GIT_HOSTING_URL_PREFIX, |
188 | + ) |
189 | + assert gitubuntu.importer.VCS_GIT_REF_VALIDATION.fullmatch(vcs_git_ref) |
190 | + assert gitubuntu.importer.VCS_GIT_COMMIT_VALIDATION.fullmatch( |
191 | + vcs_git_commit, |
192 | + ) |
193 | + return cls( |
194 | + vcs_git=vcs_git, |
195 | + vcs_git_ref=vcs_git_ref, |
196 | + vcs_git_commit=vcs_git_commit, |
197 | + commit=commit, |
198 | + remote_name=remote_name, |
199 | + local_ref_name=ref.name, |
200 | + local_tracking_ref_name= |
201 | + 'refs/remotes/%s/%s' % (remote_name, branch_name), |
202 | + ) |
203 | + |
204 | + |
205 | +def fetch(repo, remote_name, check=True, verbose_on_failure=True): |
206 | + """Fetch from a remote using its default refspec |
207 | + |
208 | + :param GitUbuntuRepository repo: the local repository to fetch into. |
209 | + :param str remote_name: the name of the remote to fetch from. |
210 | + :param bool check: passed through to GitUbuntuRepository.git_run(). |
211 | + :param bool verbose_on_failure: passed through to |
212 | + GitUbuntuRepository.git_run(). |
213 | + :raises subprocess.CalledProcessError: if the git call fails. |
214 | + """ |
215 | + # pygit2's fetch method seems to demand authentication even though none is |
216 | + # needed in the common case that ssh keys are in use, so we call the CLI |
217 | + # instead |
218 | + repo.git_run( |
219 | + ['fetch', remote_name], |
220 | + stdout=sys.stderr, # we do want the user to see this output, but |
221 | + # cannot send it to stdout as that would interfere |
222 | + # with the output of the push-for-upload command |
223 | + # itself that needs to be captured by the user to |
224 | + # pass to their build command. |
225 | + stderr=None, # we do want the user to see this output |
226 | + # If the remote repository does not exist (eg. it will be created by |
227 | + # the push) then Launchpad will prompt to try authentication, but that |
228 | + # will have no meaning for the user, so we don't want a prompt and |
229 | + # instead the fetch call should fail. |
230 | + env={'GIT_TERMINAL_PROMPT': '0'}, |
231 | + check=check, |
232 | + verbose_on_failure=verbose_on_failure, |
233 | + ) |
234 | + |
235 | + |
236 | def push( |
237 | repo, |
238 | - remote=None, |
239 | - branch=None, |
240 | + remote_name, |
241 | + local_ref_name, |
242 | + force_push=False, |
243 | ): |
244 | - """Push to a remote and return suitable rich history reference headers |
245 | + """Push a ref to a remote |
246 | |
247 | :param GitUbuntuRepository repo: the local repository to push from |
248 | - :param str remote: the name of the remote to push to. If None, then use |
249 | - repo.lp_user. |
250 | - :param str branch: the name of the local branch to push. If None, use what |
251 | - HEAD points to. |
252 | - :rtype: dict |
253 | - :returns: a dict providing the additional key/value metadata to add to the |
254 | - changes file that references the commit just pushed as the commit to |
255 | - use for rich history import when later seen by the git-ubuntu importer. |
256 | + :param str remote_name: the name of the remote to push to. |
257 | + :param str local_ref_name: the name of the local ref to push. The same ref |
258 | + will be pushed to the remote end. Example: "refs/heads/feature". |
259 | + :param bool force_push: if a force push should be used |
260 | + :raises subprocess.CalledProcessError: if the git call fails. |
261 | """ |
262 | - if not remote: |
263 | - remote = repo.lp_user |
264 | - |
265 | - if branch: |
266 | - commit, ref = repo.raw_repo.resolve_refish(branch) |
267 | - # ref might be None if the given branch parameter isn't actually a |
268 | - # branch (eg. is a commit) |
269 | - assert ref |
270 | - else: |
271 | - assert not repo.raw_repo.head_is_detached |
272 | - ref = repo.raw_repo.head |
273 | - commit = ref.peel(pygit2.Commit) |
274 | - |
275 | - pygit2_remote = repo.raw_repo.remotes[remote] |
276 | - |
277 | - # Rewrite rule for VCS URLs. See LP: #1942985 |
278 | - vcs_git = URL_REWRITE[0].sub(URL_REWRITE[1], pygit2_remote.url) |
279 | - |
280 | - vcs_git_ref = ref.name |
281 | - vcs_git_commit = str(commit.id) |
282 | - |
283 | - # Pre-empt any failure with later input validation of the headers we will |
284 | - # generate after we push. If these assertions fail, then the later import |
285 | - # will refuse to accept this rich history, so better that we fail now so |
286 | - # the uploader knows there's some kind of problem, rather than later when |
287 | - # the importer will silently throw away the rich history that the uploader |
288 | - # pushed. |
289 | - # |
290 | - # However, we don't know of any actual case when this might happen, so |
291 | - # these are assertions rather than fully UX-compliant error paths. |
292 | - assert gitubuntu.importer.VCS_GIT_URL_VALIDATION.fullmatch(vcs_git) |
293 | - assert vcs_git.startswith( |
294 | - gitubuntu.importer.LAUNCHPAD_GIT_HOSTING_URL_PREFIX, |
295 | - ) |
296 | - assert gitubuntu.importer.VCS_GIT_REF_VALIDATION.fullmatch(vcs_git_ref) |
297 | - assert gitubuntu.importer.VCS_GIT_COMMIT_VALIDATION.fullmatch( |
298 | - vcs_git_commit, |
299 | - ) |
300 | - |
301 | # pygit2's push method seems to demand authentication even though none is |
302 | - # needed in the common case that ssh keys are in use |
303 | + # needed in the common case that ssh keys are in use, so we call the CLI |
304 | + # instead |
305 | + args = ['push', remote_name] |
306 | + if force_push: |
307 | + args.append('--force') |
308 | + args.append('%s:%s' % (local_ref_name, local_ref_name)) |
309 | repo.git_run( |
310 | - ['push', remote, '%s:%s' % (ref.name, ref.name)], |
311 | + args, |
312 | stdout=sys.stderr, # we do want the user to see this output, but |
313 | # cannot send it to stdout as that would interfere |
314 | # with the output of the push-for-upload command |
315 | @@ -152,15 +288,117 @@ def push( |
316 | stderr=None, # we do want the user to see this output |
317 | ) |
318 | |
319 | - return { |
320 | - 'Vcs-Git': vcs_git, |
321 | - 'Vcs-Git-Ref': vcs_git_ref, |
322 | - 'Vcs-Git-Commit': vcs_git_commit, |
323 | - } |
324 | + |
325 | +def ref_has_commit(repo, ref, commit): |
326 | + """Determine if a commit can be reached from a given reference |
327 | + |
328 | + :param GitUbuntuRepository repo: the repository to consider |
329 | + :param pygit2.Reference ref: which reference to look in |
330 | + :param pygit2.Commit commit: which commit to find |
331 | + :rtype: bool |
332 | + :returns: True if the commit can be reached; False otherwise |
333 | + |
334 | + This differs from pygit2.Repository.descendant_of() because that method |
335 | + would return False for the commit the ref points to. |
336 | + """ |
337 | + ref_tip = ref.peel(pygit2.Commit) |
338 | + return ( |
339 | + ref_tip.id == commit.id |
340 | + or repo.raw_repo.descendant_of(ref_tip.id, commit.id) |
341 | + ) |
342 | |
343 | |
344 | -def mangle_changes(changes_file_path, headers): |
345 | - """Mangle a changes file with the given headers |
346 | +def mod_changes_file(changes_file_path, replacements): |
347 | + """Modify an existing changes file |
348 | + |
349 | + :param str changes_file_path: the changes file to modify |
350 | + :param dict(str, str) replacements: the replacement keys and values |
351 | + """ |
352 | + modded_changes_file_path = f'{changes_file_path}_modded' |
353 | + |
354 | + with open(changes_file_path) as f: |
355 | + changes = debian.deb822.Changes(f) |
356 | + changes.update(replacements) |
357 | + with open(modded_changes_file_path, 'wb') as f: |
358 | + changes.dump(f) |
359 | + os.rename(modded_changes_file_path, changes_file_path) |
360 | + |
361 | + |
362 | +def ensure_in_remote( |
363 | + repo, |
364 | + remote_name, |
365 | + local_ref_name, |
366 | + local_tracking_ref_name, |
367 | + commit, |
368 | + force_push=False, |
369 | +): |
370 | + """Ensure that a local ref and commit are present in a remote |
371 | + |
372 | + Fetch the default refspec(s) from a remote, check if a remote tracking |
373 | + branch contains a commit, and push a local ref if it does not. |
374 | + |
375 | + :param GitUbuntuRepository repo: the git repository to use |
376 | + :param str remote_name: the name of the remote |
377 | + :param str local_ref_name: the name of the local ref |
378 | + :param str local_tracking_ref_name: the name of local tracking branch of |
379 | + the remote ref |
380 | + :param pygit2.Commit commit: the commit |
381 | + :param bool force_push: if a force push should be used |
382 | + :raises ForcePushRequired: if a force push is required but not requested |
383 | + """ |
384 | + # Do a best-effort attempt to get the remote tracking branch updated. If |
385 | + # this doesn't succeed, then we will just proceed as if our understanding |
386 | + # of the state of the remote branch is up-to-date. Therefore we can ignore |
387 | + # failures. It's possible that the user has force-pushed the remote branch |
388 | + # backwards and not updated the remote tracking branch and the fetch also |
389 | + # fails. In this case, we'll think we don't need to push, and |
390 | + # prepare-upload will supply rich history that is not present in the |
391 | + # remote. This seems unlikely and an acceptable risk. |
392 | + fetch(repo, remote_name, check=False, verbose_on_failure=False) |
393 | + try: |
394 | + local_tracking_ref = repo.raw_repo.lookup_reference( |
395 | + local_tracking_ref_name |
396 | + ) |
397 | + except KeyError: |
398 | + needs_push = True |
399 | + else: |
400 | + needs_push = not ref_has_commit(repo, local_tracking_ref, commit) |
401 | + |
402 | + if needs_push: |
403 | + local_ref = repo.raw_repo.lookup_reference(local_ref_name) |
404 | + local_ref_commit_id = local_ref.peel(pygit2.Commit).id |
405 | + local_tracking_ref_id = local_tracking_ref.peel(pygit2.Commit).id |
406 | + |
407 | + # If these were the same, then a push wouldn't be required |
408 | + assert local_ref_commit_id != local_tracking_ref_id |
409 | + |
410 | + is_fast_forward = repo.raw_repo.descendant_of( |
411 | + local_ref_commit_id, |
412 | + local_tracking_ref_id, |
413 | + ) |
414 | + if not is_fast_forward and not force_push: |
415 | + raise ForcePushRequired() |
416 | + |
417 | + if needs_push: |
418 | + push( |
419 | + repo=repo, |
420 | + remote_name=remote_name, |
421 | + local_ref_name=local_ref_name, |
422 | + force_push=force_push, |
423 | + ) |
424 | + |
425 | + |
426 | +def mangle_changes( |
427 | + repo, |
428 | + changes_file_path, |
429 | + remote_name=None, |
430 | + branch_name=None, |
431 | + force_push=False, |
432 | +): |
433 | + """Pythonic API entry point to the "prepare-upload mangle" subcommand |
434 | + |
435 | + Ensure rich history is present in a remote for adoption by the importer, |
436 | + then mangle a changes file with the required headers. |
437 | |
438 | Replace the changes file at the given path with one that has the given |
439 | headers overridden. If the changes file was previously signed, the |
440 | @@ -169,44 +407,116 @@ def mangle_changes(changes_file_path, headers): |
441 | To avoid the risk of ending up with corrupted changes files, this writes to |
442 | a new file and then does an atomic rename over the old path. |
443 | |
444 | + :param GitUbuntuRepository repo: the git repository to use |
445 | :param str changes_file_path: path to the changes file to mangle |
446 | - :param dict headers: headers to override with |
447 | + :param str remote_name: the name of the remote to use, or None to use the |
448 | + remote named after the user's Launchpad username (as added by "git |
449 | + clone"). |
450 | + :param str branch_name: the name of the branch to use (both local and |
451 | + remote), or None for the name of the currently checked-out branch. |
452 | + :param bool force_push: if a force push should be used |
453 | :returns: None |
454 | + :raises ForcePushRequired: if a force push is required but not requested |
455 | """ |
456 | - mangled_changes_file_path = f'{changes_file_path}_mangled' |
457 | + parameters = Parameters.from_repo( |
458 | + repo=repo, |
459 | + remote_name=remote_name, |
460 | + branch_name=branch_name, |
461 | + ) |
462 | + ensure_in_remote( |
463 | + repo=repo, |
464 | + remote_name=parameters.remote_name, |
465 | + local_ref_name=parameters.local_ref_name, |
466 | + local_tracking_ref_name=parameters.local_tracking_ref_name, |
467 | + commit=parameters.commit, |
468 | + force_push=force_push, |
469 | + ) |
470 | + mod_changes_file( |
471 | + changes_file_path=changes_file_path, |
472 | + replacements=parameters.changes_file_headers, |
473 | + ) |
474 | |
475 | - with open(changes_file_path) as f: |
476 | - changes = debian.deb822.Changes(f) |
477 | - for k, v in headers.items(): |
478 | - changes[k] = v |
479 | - with open(mangled_changes_file_path, 'wb') as f: |
480 | - changes.dump(f) |
481 | - os.rename(mangled_changes_file_path, changes_file_path) |
482 | + |
483 | +def establish_args(repo, remote_name=None, branch_name=None, force_push=False): |
484 | + """Pythonic API entry point to the "prepare-upload args" subcommand |
485 | + |
486 | + Ensure rich history is present in a remote for adoption by the importer, |
487 | + then return the parameters that contain the arguments required to arrange |
488 | + for a changes file to be generated with the required headers. |
489 | + |
490 | + :param GitUbuntuRepository repo: the git repository to use |
491 | + :param str remote_name: the name of the remote to use, or None to use the |
492 | + remote named after the user's Launchpad username (as added by "git |
493 | + clone"). |
494 | + :param str branch_name: the name of the branch to use (both local and |
495 | + remote), or None for the name of the currently checked-out branch. |
496 | + :param bool force_push: if a force push should be used |
497 | + :rtype: Parameters |
498 | + :returns: parameters that were used as determined by heuristics and the |
499 | + user |
500 | + :raises ForcePushRequired: if a force push is required but not requested |
501 | + """ |
502 | + parameters = Parameters.from_repo( |
503 | + repo=repo, |
504 | + remote_name=remote_name, |
505 | + branch_name=branch_name, |
506 | + ) |
507 | + ensure_in_remote( |
508 | + repo=repo, |
509 | + remote_name=parameters.remote_name, |
510 | + local_ref_name=parameters.local_ref_name, |
511 | + local_tracking_ref_name=parameters.local_tracking_ref_name, |
512 | + commit=parameters.commit, |
513 | + force_push=force_push, |
514 | + ) |
515 | + return parameters |
516 | |
517 | |
518 | -def cli_printargs(args): |
519 | - """Entry point for the args subcommand""" |
520 | +def cli_printargs(args): # pragma: no cover |
521 | + """CLI entry point for the args subcommand""" |
522 | try: |
523 | repo = gitubuntu.git_repository.GitUbuntuRepository('.') |
524 | - headers = push( |
525 | + parameters = establish_args( |
526 | repo=repo, |
527 | - remote=args.remote, |
528 | - branch=args.branch, |
529 | + remote_name=args.remote, |
530 | + branch_name=args.branch, |
531 | + force_push=args.force_push, |
532 | ) |
533 | + except ForcePushRequired: |
534 | + print("--git-ubuntu-prepare-upload-args-failed") |
535 | + print( |
536 | + "git-ubuntu: the remote branch cannot be fast-forwarded." |
537 | + " Do you need --force-push?", |
538 | + file=sys.stderr, |
539 | + ) |
540 | + return 2 |
541 | except: |
542 | print("--git-ubuntu-prepare-upload-args-failed") |
543 | raise |
544 | else: |
545 | - print(OUTPUT_FORMATS[args.output_format].format(**headers)) |
546 | + print( |
547 | + OUTPUT_FORMATS[args.output_format].format( |
548 | + **parameters.changes_file_headers |
549 | + ) |
550 | + ) |
551 | return 0 |
552 | |
553 | |
554 | -def cli_manglechanges(args): |
555 | - """Entry point for the mangle subcommand""" |
556 | +def cli_manglechanges(args): # pragma: no cover |
557 | + """CLI entry point for the mangle subcommand""" |
558 | repo = gitubuntu.git_repository.GitUbuntuRepository('.') |
559 | - headers = push( |
560 | - repo=repo, |
561 | - remote=args.remote, |
562 | - branch=args.branch, |
563 | - ) |
564 | - mangle_changes(changes_file_path=args.changes_file_path, headers=headers) |
565 | + try: |
566 | + mangle_changes( |
567 | + repo=repo, |
568 | + remote_name=args.remote, |
569 | + branch_name=args.branch, |
570 | + changes_file_path=args.changes_file_path, |
571 | + force_push=args.force_push, |
572 | + ) |
573 | + except ForcePushRequired: |
574 | + print( |
575 | + "git-ubuntu: the remote branch cannot be fast-forwarded." |
576 | + " Do you need --force-push?", |
577 | + file=sys.stderr, |
578 | + ) |
579 | + return 2 |
580 | diff --git a/gitubuntu/prepare_upload_test.py b/gitubuntu/prepare_upload_test.py |
581 | index a1b4911..979f627 100644 |
582 | --- a/gitubuntu/prepare_upload_test.py |
583 | +++ b/gitubuntu/prepare_upload_test.py |
584 | @@ -1,12 +1,14 @@ |
585 | +import re |
586 | import sys |
587 | import unittest.mock |
588 | +from unittest.mock import patch |
589 | |
590 | import pygit2 |
591 | import pytest |
592 | |
593 | from gitubuntu.git_repository import GitUbuntuRepository |
594 | from gitubuntu.repo_builder import Commit, Placeholder, Repo |
595 | -from gitubuntu.test_fixtures import repo |
596 | +from gitubuntu.test_fixtures import repo, pygit2_repo |
597 | |
598 | import gitubuntu.prepare_upload as target |
599 | |
600 | @@ -17,18 +19,19 @@ def populated_repo(repo): |
601 | suitable data for the purposes of the tests in this module""" |
602 | Repo( |
603 | commits=[ |
604 | - Commit(name='master', message='master'), |
605 | + Commit(name='main', message='main'), |
606 | Commit(name='other', message='other'), |
607 | ], |
608 | branches={ |
609 | - 'master': Placeholder('master'), |
610 | + 'main': Placeholder('main'), |
611 | 'other': Placeholder('other'), |
612 | }, |
613 | ).write(repo.raw_repo) |
614 | + repo.raw_repo.checkout('refs/heads/main') |
615 | yield repo |
616 | |
617 | |
618 | -def test_push_with_branch(populated_repo): |
619 | +def test_establish_args_with_branch(populated_repo): |
620 | """Pushes and outputs correct fields when a branch name is specified |
621 | |
622 | :param GitUbuntuRepository populated_repo: fixture providing a temporary |
623 | @@ -41,9 +44,13 @@ def test_push_with_branch(populated_repo): |
624 | 'https://git.launchpad.net/test', |
625 | ) |
626 | |
627 | - with unittest.mock.patch.object(repo, 'git_run') as mock_run: |
628 | - result = target.push(repo=repo, remote='testremote', branch='other') |
629 | - mock_run.assert_called_once_with( |
630 | + with patch.object(repo, 'git_run') as mock_run: |
631 | + result = target.establish_args( |
632 | + repo=repo, |
633 | + remote_name='testremote', |
634 | + branch_name='other', |
635 | + ) |
636 | + mock_run.assert_called_with( |
637 | [ |
638 | 'push', |
639 | 'testremote', |
640 | @@ -60,10 +67,10 @@ def test_push_with_branch(populated_repo): |
641 | 'Vcs-Git-Ref': 'refs/heads/other', |
642 | 'Vcs-Git-Commit': str(commit_hash), |
643 | } |
644 | - assert result == expected_result |
645 | + assert result.changes_file_headers == expected_result |
646 | |
647 | |
648 | -def test_push_without_branch(populated_repo): |
649 | +def test_establish_args_without_branch(populated_repo): |
650 | """Pushes and outputs correct fields when no branch is specified |
651 | |
652 | :param GitUbuntuRepository populated_repo: fixture providing a temporary |
653 | @@ -77,9 +84,9 @@ def test_push_without_branch(populated_repo): |
654 | 'https://git.launchpad.net/test', |
655 | ) |
656 | |
657 | - with unittest.mock.patch.object(repo, 'git_run') as mock_run: |
658 | - result = target.push(repo=repo, remote='testremote') |
659 | - mock_run.assert_called_once_with( |
660 | + with patch.object(repo, 'git_run') as mock_run: |
661 | + result = target.establish_args(repo=repo, remote_name='testremote') |
662 | + mock_run.assert_called_with( |
663 | [ |
664 | 'push', |
665 | 'testremote', |
666 | @@ -96,10 +103,10 @@ def test_push_without_branch(populated_repo): |
667 | 'Vcs-Git-Ref': 'refs/heads/other', |
668 | 'Vcs-Git-Commit': str(commit_hash), |
669 | } |
670 | - assert result == expected_result |
671 | + assert result.changes_file_headers == expected_result |
672 | |
673 | |
674 | -def test_push_without_remote(populated_repo): |
675 | +def test_establish_args_without_remote(populated_repo): |
676 | """Pushes and outputs correct fields when no remote is specified |
677 | |
678 | :param GitUbuntuRepository populated_repo: fixture providing a temporary |
679 | @@ -112,29 +119,29 @@ def test_push_without_remote(populated_repo): |
680 | repo = GitUbuntuRepository(repo.raw_repo.workdir) |
681 | |
682 | repo.raw_repo.remotes.create('testuser', 'https://git.launchpad.net/~test') |
683 | - with unittest.mock.patch.object(repo, 'git_run') as mock_run: |
684 | - result = target.push(repo=repo) |
685 | - mock_run.assert_called_once_with( |
686 | + with patch.object(repo, 'git_run') as mock_run: |
687 | + result = target.establish_args(repo=repo) |
688 | + mock_run.assert_called_with( |
689 | [ |
690 | 'push', |
691 | 'testuser', |
692 | - 'refs/heads/master:refs/heads/master', |
693 | + 'refs/heads/main:refs/heads/main', |
694 | ], |
695 | stdout=unittest.mock.ANY, |
696 | stderr=None, |
697 | ) |
698 | |
699 | - master_ref = repo.raw_repo.lookup_reference('refs/heads/master') |
700 | - commit_hash = master_ref.peel(pygit2.Commit).id |
701 | + main_ref = repo.raw_repo.lookup_reference('refs/heads/main') |
702 | + commit_hash = main_ref.peel(pygit2.Commit).id |
703 | expected_result = { |
704 | 'Vcs-Git': 'https://git.launchpad.net/~test', |
705 | - 'Vcs-Git-Ref': 'refs/heads/master', |
706 | + 'Vcs-Git-Ref': 'refs/heads/main', |
707 | 'Vcs-Git-Commit': str(commit_hash), |
708 | } |
709 | - assert result == expected_result |
710 | + assert result.changes_file_headers == expected_result |
711 | |
712 | |
713 | -def test_push_stdout(populated_repo): |
714 | +def test_establish_args_stdout(populated_repo): |
715 | """git push output is written to the terminal |
716 | |
717 | The user expects to see the output of "git push" on the terminal. However, |
718 | @@ -150,21 +157,37 @@ def test_push_stdout(populated_repo): |
719 | 'testremote', |
720 | 'https://git.launchpad.net/test', |
721 | ) |
722 | - with unittest.mock.patch.object(repo, 'git_run') as mock_run: |
723 | - result = target.push( |
724 | + with patch.object(repo, 'git_run') as mock_run: |
725 | + result = target.establish_args( |
726 | repo=repo, |
727 | - remote='testremote', |
728 | - branch='other', |
729 | + remote_name='testremote', |
730 | + branch_name='other', |
731 | ) |
732 | assert mock_run.call_args.kwargs['stdout'] is sys.stderr |
733 | assert mock_run.call_args.kwargs['stderr'] is None |
734 | |
735 | |
736 | -@unittest.mock.patch("builtins.print") |
737 | -@unittest.mock.patch("gitubuntu.prepare_upload.push") |
738 | -def test_printargs_failure(mock_push, mock_print, populated_repo, monkeypatch): |
739 | +@patch("builtins.print") |
740 | +@patch("gitubuntu.prepare_upload.establish_args") |
741 | +def test_establish_args_failure( |
742 | + mock_establish_args, |
743 | + mock_print, |
744 | + populated_repo, |
745 | + monkeypatch, |
746 | +): |
747 | + """On failure, the CLI prints an impossible argument |
748 | + |
749 | + :param unittest.mock.Mock mock_establish_args: the mocked out |
750 | + establish_args function |
751 | + :param unittest.mock.Mock mock_print: the mocked out print function |
752 | + :param GitUbuntuRepository populated_repo: fixture providing a temporary |
753 | + GitUbuntuRepository populated with our standard test data. |
754 | + :param monkeypatch: the standard pytest monkeypatch fixture |
755 | + |
756 | + See: LP #1942865 |
757 | + """ |
758 | monkeypatch.chdir(populated_repo.raw_repo.workdir) |
759 | - mock_push.side_effect = RuntimeError("Mock failure") |
760 | + mock_establish_args.side_effect = RuntimeError("Mock failure") |
761 | with pytest.raises(RuntimeError): |
762 | target.cli_printargs(unittest.mock.Mock( |
763 | remote=None, |
764 | @@ -180,19 +203,24 @@ def test_printargs_failure(mock_push, mock_print, populated_repo, monkeypatch): |
765 | ('git+ssh://user@git.launchpad.net/test',), |
766 | ('ssh://user@git.launchpad.net/test',), |
767 | ]) |
768 | -def test_push_rewrites_lp_ssh_url(populated_repo, remote_url): |
769 | - """Rewrites Vcs-Git to an https: one when git+ssh: or ssh: is used with LP |
770 | +def test_establish_args_rewrites_lp_ssh_url(populated_repo, remote_url): |
771 | + """Rewrites Vcs-Git: to an https: one when git+ssh: or ssh: is used with LP |
772 | |
773 | See LP: #1942985 |
774 | |
775 | :param GitUbuntuRepository populated_repo: fixture providing a temporary |
776 | GitUbuntuRepository populated with our standard test data. |
777 | + :param str remote_url: the URL that we will test is rewritten |
778 | """ |
779 | repo = populated_repo |
780 | repo.raw_repo.remotes.create('testremote', remote_url) |
781 | - with unittest.mock.patch.object(repo, 'git_run') as mock_run: |
782 | - result = target.push(repo=repo, remote='testremote', branch='other') |
783 | - mock_run.assert_called_once_with( |
784 | + with patch.object(repo, 'git_run') as mock_run: |
785 | + result = target.establish_args( |
786 | + repo=repo, |
787 | + remote_name='testremote', |
788 | + branch_name='other', |
789 | + ) |
790 | + mock_run.assert_called_with( |
791 | [ |
792 | 'push', |
793 | 'testremote', |
794 | @@ -209,7 +237,7 @@ def test_push_rewrites_lp_ssh_url(populated_repo, remote_url): |
795 | 'Vcs-Git-Ref': 'refs/heads/other', |
796 | 'Vcs-Git-Commit': str(commit_hash), |
797 | } |
798 | - assert result == expected_result |
799 | + assert result.changes_file_headers == expected_result |
800 | |
801 | |
802 | FAKE_SIGNED_CHANGES_FILE = """-----BEGIN PGP SIGNED MESSAGE----- |
803 | @@ -230,6 +258,7 @@ def fake_changes_file_path(tmpdir): |
804 | This fixture does no cleanup on the assumption that the automatic cleanup |
805 | of the tmpdir fixture it uses is sufficient. |
806 | |
807 | + :param tmpdir: the standard pytest tmpdir fixture |
808 | :rtype: str |
809 | :returns: the path to our standard fake changes file in a temporary |
810 | directory |
811 | @@ -240,26 +269,253 @@ def fake_changes_file_path(tmpdir): |
812 | return changes_file_path |
813 | |
814 | |
815 | -def test_mangle_changes(fake_changes_file_path): |
816 | - """mangle_changes mangles our standard fake changes file as expected |
817 | +def test_mangle_changes(populated_repo, fake_changes_file_path): |
818 | + """Pushes and adds correct fields |
819 | + |
820 | + :param GitUbuntuRepository populated_repo: fixture providing a temporary |
821 | + GitUbuntuRepository populated with our standard test data. |
822 | + :param str fake_changes_file_path: our test changes file fixture |
823 | + """ |
824 | + repo = populated_repo |
825 | + |
826 | + repo.raw_repo.checkout('refs/heads/other') |
827 | + repo.raw_repo.remotes.create( |
828 | + 'testremote', |
829 | + 'https://git.launchpad.net/test', |
830 | + ) |
831 | + |
832 | + with patch.object(repo, 'git_run') as mock_run: |
833 | + target.mangle_changes( |
834 | + repo=repo, |
835 | + remote_name='testremote', |
836 | + branch_name='main', |
837 | + changes_file_path=fake_changes_file_path, |
838 | + ) |
839 | + mock_run.assert_called_with( |
840 | + [ |
841 | + 'push', |
842 | + 'testremote', |
843 | + 'refs/heads/main:refs/heads/main', |
844 | + ], |
845 | + stdout=unittest.mock.ANY, |
846 | + stderr=None, |
847 | + ) |
848 | + |
849 | + main_ref = repo.raw_repo.lookup_reference('refs/heads/main') |
850 | + commit_hash = main_ref.peel(pygit2.Commit).id |
851 | + expected_result = { |
852 | + 'vcs_git': 'https://git.launchpad.net/test', |
853 | + 'vcs_git_ref': 'refs/heads/main', |
854 | + 'vcs_git_commit': str(commit_hash), |
855 | + } |
856 | + with open(fake_changes_file_path, 'r') as f: |
857 | + data = f.read() |
858 | + assert sorted(data.splitlines()) == [ |
859 | + "Format: 1.8", |
860 | + "Source: test", |
861 | + f"Vcs-Git-Commit: {commit_hash!s}", |
862 | + "Vcs-Git-Ref: refs/heads/main", |
863 | + "Vcs-Git: https://git.launchpad.net/test", |
864 | + ] |
865 | + |
866 | + |
867 | +def test_mod_changes_file(fake_changes_file_path): |
868 | + """mod_changes_file mangles our standard fake changes file as expected |
869 | |
870 | Note that this includes stripping of the signature. |
871 | + |
872 | + :param str fake_changes_file_path: our test changes file fixture |
873 | """ |
874 | - target.mangle_changes(fake_changes_file_path, {'Added': 'yes'}) |
875 | + target.mod_changes_file( |
876 | + changes_file_path=fake_changes_file_path, |
877 | + replacements={'Added': 'yes'}, |
878 | + ) |
879 | with open(fake_changes_file_path, 'r') as f: |
880 | data = f.read() |
881 | assert data == "Format: 1.8\nSource: test\nAdded: yes\n" |
882 | |
883 | |
884 | -def test_mangle_changes_idempotent(fake_changes_file_path): |
885 | +def test_mod_changes_file_idempotent(fake_changes_file_path): |
886 | """mangle_changes operates idempotently |
887 | |
888 | If run twice, the output should still be as expected. Note that this has |
889 | the effect of verifying that it will also work correctly when the changes |
890 | file is unsigned, since that's the result of the first call. |
891 | + |
892 | + :param str fake_changes_file_path: our test changes file fixture |
893 | """ |
894 | - target.mangle_changes(fake_changes_file_path, {'Added': 'yes'}) |
895 | - target.mangle_changes(fake_changes_file_path, {'Added': 'yes'}) |
896 | + target.mod_changes_file(fake_changes_file_path, {'Added': 'yes'}) |
897 | + target.mod_changes_file(fake_changes_file_path, {'Added': 'yes'}) |
898 | with open(fake_changes_file_path, 'r') as f: |
899 | data = f.read() |
900 | assert data == "Format: 1.8\nSource: test\nAdded: yes\n" |
901 | + |
902 | + |
903 | +@patch('gitubuntu.importer.LAUNCHPAD_GIT_HOSTING_URL_PREFIX', '') |
904 | +@patch('gitubuntu.importer.VCS_GIT_URL_VALIDATION', re.compile(r'.*')) |
905 | +def test_fetch_gets_called(populated_repo, pygit2_repo): |
906 | + """We fetch from the repository during a establish_args operation |
907 | + |
908 | + :param GitUbuntuRepository populated_repo: fixture providing a temporary |
909 | + GitUbuntuRepository populated with our standard test data. |
910 | + :param pygit.Repository pygit2_repo: our empty pygit2 repository fixture. |
911 | + """ |
912 | + # Note that we cannot use a repo instance twice, even if one is via |
913 | + # populated_repo, because pytest will give us only one instance. A |
914 | + # convenient workaround is that the remote repo doesn't need to be a |
915 | + # GitUbuntuRepository and so the pygit2_repo fixture will do as it is |
916 | + # independent. We can assert they are definitely different: |
917 | + assert populated_repo.raw_repo is not pygit2_repo |
918 | + |
919 | + remote_repo = pygit2_repo |
920 | + local_repo = populated_repo |
921 | + |
922 | + # Allow the push even for our non-bare "remote" test repo |
923 | + pygit2_repo.config['receive.denyCurrentBranch'] = 'ignore' |
924 | + |
925 | + local_repo.raw_repo.remotes.create('test-remote', pygit2_repo.path) |
926 | + local_repo.git_run(["push", "test-remote", "main"]) |
927 | + |
928 | + with patch.object(local_repo, 'git_run') as mock_run: |
929 | + target.establish_args( |
930 | + repo=local_repo, |
931 | + remote_name='test-remote', |
932 | + branch_name='main', |
933 | + ) |
934 | + mock_run.assert_called_with( |
935 | + ['fetch', 'test-remote'], |
936 | + stdout=unittest.mock.ANY, |
937 | + stderr=None, |
938 | + env=unittest.mock.ANY, |
939 | + check=False, |
940 | + verbose_on_failure=False, |
941 | + ) |
942 | + |
943 | + |
944 | +@patch('gitubuntu.importer.LAUNCHPAD_GIT_HOSTING_URL_PREFIX', '') |
945 | +@patch('gitubuntu.importer.VCS_GIT_URL_VALIDATION', re.compile(r'.*')) |
946 | +def test_no_push_when_already_present(populated_repo, pygit2_repo): |
947 | + """If the rich history is already available, we do not try to push again |
948 | + |
949 | + :param GitUbuntuRepository populated_repo: fixture providing a temporary |
950 | + GitUbuntuRepository populated with our standard test data. |
951 | + :param pygit.Repository pygit2_repo: our empty pygit2 repository fixture. |
952 | + """ |
953 | + # Note that we cannot use a repo instance twice, even if one is via |
954 | + # populated_repo, because pytest will give us only one instance. A |
955 | + # convenient workaround is that the remote repo doesn't need to be a |
956 | + # GitUbuntuRepository and so the pygit2_repo fixture will do as it is |
957 | + # independent. We can assert they are definitely different: |
958 | + assert populated_repo.raw_repo is not pygit2_repo |
959 | + |
960 | + remote_repo = pygit2_repo |
961 | + local_repo = populated_repo |
962 | + |
963 | + # Allow the push even for our non-bare "remote" test repo |
964 | + pygit2_repo.config['receive.denyCurrentBranch'] = 'ignore' |
965 | + |
966 | + local_repo.raw_repo.remotes.create('test-remote', pygit2_repo.path) |
967 | + local_repo.git_run(["push", "test-remote", "main"]) |
968 | + |
969 | + with patch.object(target, 'push') as mock_push: |
970 | + target.establish_args( |
971 | + repo=local_repo, |
972 | + remote_name='test-remote', |
973 | + branch_name='main', |
974 | + ) |
975 | + mock_push.assert_not_called() |
976 | + |
977 | + |
978 | +@patch('gitubuntu.importer.LAUNCHPAD_GIT_HOSTING_URL_PREFIX', '') |
979 | +@patch('gitubuntu.importer.VCS_GIT_URL_VALIDATION', re.compile(r'.*')) |
980 | +def test_rejects_force_push(populated_repo, pygit2_repo): |
981 | + """If the remote branch cannot be fast-forwarded and we didn't request a |
982 | + force push, then an exception is raised |
983 | + |
984 | + :param GitUbuntuRepository populated_repo: fixture providing a temporary |
985 | + GitUbuntuRepository populated with our standard test data. |
986 | + :param pygit.Repository pygit2_repo: our empty pygit2 repository fixture. |
987 | + """ |
988 | + # Note that we cannot use a repo instance twice, even if one is via |
989 | + # populated_repo, because pytest will give us only one instance. A |
990 | + # convenient workaround is that the remote repo doesn't need to be a |
991 | + # GitUbuntuRepository and so the pygit2_repo fixture will do as it is |
992 | + # independent. We can assert they are definitely different: |
993 | + assert populated_repo.raw_repo is not pygit2_repo |
994 | + |
995 | + remote_repo = pygit2_repo |
996 | + local_repo = populated_repo |
997 | + |
998 | + # Write something to the remote main branch so it cannot be fast-forwarded |
999 | + # with what we have locally |
1000 | + Repo( |
1001 | + commits=[ |
1002 | + Commit(name='main', message='mutated'), |
1003 | + ], |
1004 | + branches={ |
1005 | + 'main': Placeholder('main'), |
1006 | + }, |
1007 | + ).write(remote_repo) |
1008 | + |
1009 | + local_repo.raw_repo.remotes.create('test-remote', pygit2_repo.path) |
1010 | + |
1011 | + with pytest.raises(target.ForcePushRequired): |
1012 | + target.establish_args( |
1013 | + repo=local_repo, |
1014 | + remote_name='test-remote', |
1015 | + branch_name='main', |
1016 | + ) |
1017 | + |
1018 | + |
1019 | +@patch('gitubuntu.importer.LAUNCHPAD_GIT_HOSTING_URL_PREFIX', '') |
1020 | +@patch('gitubuntu.importer.VCS_GIT_URL_VALIDATION', re.compile(r'.*')) |
1021 | +def test_performs_force_push(populated_repo, pygit2_repo): |
1022 | + """If the remote branch cannot be fast-forwarded and we did request a force |
1023 | + push, then the remote branch is updated |
1024 | + |
1025 | + :param GitUbuntuRepository populated_repo: fixture providing a temporary |
1026 | + GitUbuntuRepository populated with our standard test data. |
1027 | + :param pygit.Repository pygit2_repo: our empty pygit2 repository fixture. |
1028 | + """ |
1029 | + # Note that we cannot use a repo instance twice, even if one is via |
1030 | + # populated_repo, because pytest will give us only one instance. A |
1031 | + # convenient workaround is that the remote repo doesn't need to be a |
1032 | + # GitUbuntuRepository and so the pygit2_repo fixture will do as it is |
1033 | + # independent. We can assert they are definitely different: |
1034 | + assert populated_repo.raw_repo is not pygit2_repo |
1035 | + |
1036 | + remote_repo = pygit2_repo |
1037 | + local_repo = populated_repo |
1038 | + |
1039 | + # Write something to the remote main branch so it cannot be fast-forwarded |
1040 | + # with what we have locally |
1041 | + Repo( |
1042 | + commits=[ |
1043 | + Commit(name='main', message='mutated'), |
1044 | + ], |
1045 | + branches={ |
1046 | + 'main': Placeholder('main'), |
1047 | + }, |
1048 | + ).write(remote_repo) |
1049 | + |
1050 | + # Allow the push even for our non-bare "remote" test repo |
1051 | + pygit2_repo.config['receive.denyCurrentBranch'] = 'ignore' |
1052 | + |
1053 | + local_repo.raw_repo.remotes.create('test-remote', pygit2_repo.path) |
1054 | + |
1055 | + target.establish_args( |
1056 | + repo=local_repo, |
1057 | + remote_name='test-remote', |
1058 | + branch_name='main', |
1059 | + force_push=True, |
1060 | + ) |
1061 | + |
1062 | + local_commit_id = ( |
1063 | + populated_repo.raw_repo.references["refs/heads/main"] |
1064 | + .peel(pygit2.Commit) |
1065 | + .id |
1066 | + ) |
1067 | + remote_commit_id = ( |
1068 | + pygit2_repo.references["refs/heads/main"].peel(pygit2.Commit).id |
1069 | + ) |
1070 | + assert local_commit_id == remote_commit_id |
1071 | diff --git a/sandbox/gu-build b/sandbox/gu-build |
1072 | index 78c6f4f..d5b2b8f 100755 |
1073 | --- a/sandbox/gu-build |
1074 | +++ b/sandbox/gu-build |
1075 | @@ -42,11 +42,8 @@ parser = argparse.ArgumentParser( |
1076 | usage='gu-build [-h] [--remote REMOTE] [--branch BRANCH]\n' |
1077 | ' [-- DPKG_BUILDPACKAGE_ARGS]') |
1078 | |
1079 | -# fixme: should be exported by gitubuntu.prepare_upload |
1080 | -parser.add_argument('--remote', type=str, help="Remote to push to") |
1081 | -parser.add_argument('--branch', type=str, help="Branch to push") |
1082 | - |
1083 | -parser.parse_args(prepare_upload_args) |
1084 | +gitubuntu.prepare_upload._add_subparser_arguments(parser) |
1085 | +parsed_prepare_upload_args = parser.parse_args(prepare_upload_args) |
1086 | |
1087 | repo = gitubuntu.git_repository.GitUbuntuRepository('.') |
1088 | |
1089 | @@ -81,10 +78,15 @@ if old_changelog.upstream_version != changelog.upstream_version: |
1090 | |
1091 | gitubuntu.exportorig.main(repo, 'HEAD', search_list) |
1092 | |
1093 | -headers = gitubuntu.prepare_upload.push(repo, prepare_upload_args) |
1094 | +parameters = gitubuntu.prepare_upload.establish_args( |
1095 | + repo, |
1096 | + remote_name=parsed_prepare_upload_args.remote, |
1097 | + branch_name=parsed_prepare_upload_args.branch, |
1098 | + force_push=parsed_prepare_upload_args.force_push, |
1099 | +) |
1100 | |
1101 | # fixme: some minor duplication with gitubuntu.prepare_upload |
1102 | -for k,v in headers.items(): |
1103 | +for k,v in parameters.changes_file_headers.items(): |
1104 | dpkg_args.append("--changes-option=-D%s=%s" % (k,v)) |
1105 | |
1106 | result = subprocess.run(['dpkg-buildpackage'] + dpkg_args) |
I'm going to do some additional testing from the snap once CI has built it.