Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update git remote URL before cloning on deploys #299

Merged
merged 1 commit into from
Aug 16, 2015

Conversation

fullyint
Copy link
Contributor

Problem: The git module doesn't always pick up on changes to the repo variable, sometimes failing to clone the new repo. Potential examples: one, two, three.

Steps to reproduce

  • run deploy.yml with default repo: [email protected]:roots/bedrock.git
  • change repo: [email protected]:roots/roots-example-project.com.git (and uncomment subtree: site)
  • run deploy.yml again

Exepected outcome: New clone should be made on second run of deploy.yml after changing repo.

Actual outcome: "Clone project files" task reports ok instead of changed and no new clone is made. The old roots/bedrock.git clone remains and is copied to new release directory, despite repo variable indicating roots/roots-example-project.com.git.

Why it happens: The git module doesn't check for updates by comparing the clone with the repo specified. Instead, it compares the clone with the clone's remote by running git ls-remote origin -h HEAD in the clone's path.

(Edit: The rest of this comment is now out-dated.)

Fix: This PR puts clones in paths that are unique to each repo. Previously, the "Steps" above only yielded a clone directly in shared/source. If this PR were merged, the "Steps" above would yield clones in
shared/source/bedrock.git-aedf427
shared/source/roots-example-project.com.git-55f77f1

Now, when the git module runs git ls-remote while checking for updates, it only ever runs it in a clone directory corresponding to the repo the user specified.

The hash at the end of the filename is created from the full path of the repo. Adding the hash ensures that a new clone will still be made when the basename stays the same but there is a change in git user and/or git host.

@louim
Copy link
Contributor

louim commented Aug 13, 2015

Note that it's actually gonna be fixed in ansible core module soon ansible/ansible-modules-core#721. I would prefer a task that just run git remote set-url origin {{the_specified_url}} before running the git module, but your solution work as well until it is patched in core.

@fullyint fullyint changed the title Clone to unique project_source_path per repo Update git remote URL before cloning on deploys Aug 13, 2015
@fullyint
Copy link
Contributor Author

Thanks @louim! That's good news. I should have researched this more thoroughly.

I like your idea better, especially because it will avoid altering the structure of the shared/source directory for existing Trellis projects. So, I switched the PR to just be a task to run git remote set-url. We can remove the task when Ansible core is updated.

- name: Update git remote URL
shell: "cd {{ project_source_path }} && git remote set-url origin {{ project_git_repo }} || :"
register: git_status

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As it is right now, the task will always be marked as changed, and will alway update the remote url, even if it hasn't changed. Something like this would prevent that and would only update the url if the project has already been created and the remote changed:

- name: Check if project folder exists
  stat: path={{ project_source_path }}
  register: git_project_path

- name: Get remote URL
  cmd: git config --get remote.origin.url 
  args:
    chdir: {{ project_source_path }}
  register: remote_origin_url
  when: git_project_path.stat.exists

- name: Update git remote URL
  cmd: git remote set-url origin {{ project_git_repo }}
  args:
    chdir: {{ project_source_path }}
  when: remote_origin_url is defined and remote_origin_url != project_git_repo

I just wrote that on top of my head, so there may be syntax errors that slipped by.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@louim you always help me improve. Thank you. Consider this task:

- name: Update git remote URL
  shell: "cd {{ project_source_path }} && git config --get remote.origin.url && git remote set-url origin {{ project_git_repo }} || :"
  register: original_remote
  changed_when: original_remote.stdout not in ['', project_git_repo]

On the surface, the task appears idempotent. It reports "changed" only if the new url differs from the old url and if the clone already exists. However, technically the set-url command always runs if the clone exists. If that is unacceptable, I could add an if-then into the shell command to conditionally run the set-url command. Or more likely, I could just let go of my drive to handle this in a single task and do three tasks like those you suggested.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fullyint I think it's just a matter of preference at this point. I usually prefer smaller, easier to understand tasks and offloading the logic as much as possible to Ansible. I think using the builtin function of Ansible over shell (when possible) improve long term maintainability and readability.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @louim in this case about small tasks. It's also better when failures happen since you'll know it's a problem with a single command instead of a part of a complex command.

@fullyint
Copy link
Contributor Author

Thanks for the additional feedback @louim and @swalkinshaw. I've revised the PR. Now the git remote url is only updated if necessary.

command: git remote set-url origin {{ project_git_repo }}
args:
chdir: "{{ project_source_path }}"
when: remote_origin_url.stdout is defined and remote_origin_url.stdout != project_git_repo
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the defined check needed? Since the above task only runs when the git project exists, how would it never have stdout?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the git project doesn't yet exist, "Get current git remote URL" skips, and its registered value is

"remote_origin_url": {
  "changed": false,
  "skipped": true
}

In this scenario, remote_origin_url.stdout is not defined, so without the defined check,
when: remote_origin_url.stdout != project_git_repo
would cause the task to fail.

My conditional states "if there is a remote url available to compare," but maybe a more intuitive alternative would be "if a git clone/project exists," like this:
when: git_project.stat.exists and remote_origin_url.stdout != project_git_repo.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fullyint yes that's more clear 👍

@fullyint
Copy link
Contributor Author

Thanks @swalkinshaw. I adjusted the when: statement for the "Update git remote URL" task.

@swalkinshaw
Copy link
Member

squash and we're good

@fullyint
Copy link
Contributor Author

squashed

swalkinshaw added a commit that referenced this pull request Aug 16, 2015
Update git remote URL before cloning on deploys
@swalkinshaw swalkinshaw merged commit b6aba1e into roots:master Aug 16, 2015
@swalkinshaw
Copy link
Member

Thanks 👍

@fullyint fullyint deleted the source-path branch August 16, 2015 05:15
tangrufus added a commit to tangrufus/trellis that referenced this pull request Jun 10, 2018
Remove obsoleted `git` remote checking tasks introduced in roots#299
because recent Ansible versions are able to detect/handle `git` remote
changes.

See: https://discourse.roots.io/t/do-we-still-need-git-remote-checking-during-deploy/12639
tangrufus added a commit to tangrufus/trellis that referenced this pull request Jul 2, 2018
Remove obsoleted `git` remote checking tasks introduced in roots#299
because recent Ansible versions are able to detect/handle `git` remote
changes.

See: https://discourse.roots.io/t/do-we-still-need-git-remote-checking-during-deploy/12639
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants