concourse / git-resource

tracks commits in a branch of a Git repository
Apache License 2.0
192 stars 288 forks source link

ignore_path option does not work as expected #33

Closed friegger closed 8 years ago

friegger commented 8 years ago

We have the following ignore paths in our pipeline.

ignore_paths:
  - releases/bosh-openstack-cpi/**
  - .final_builds/**
  - docs/**
  - README.md

We have tested the glob patterns locally with the following command which resembles what the check does in the git-resource:

$ git log --format='%H' -- . ':!.final_builds/**' ':!releases/**' ':!docs/**'

Unfortunately it does not filter expected commits: https://main.bosh-ci.cf-app.com/pipelines/bosh-openstack-cpi/resources/bosh-cpi-release (Concours apparently runs in version 0.71.1)

Commits that should not be part of the list are the following: 1ff876e53c2addd4cc2a4b54eaf20e30b7e2fb19 774ec6f200df0a93b3f669cf9bdb4b6d36f4429f

Strange thing is, in our internal concourse (currently 0.71.0) we have the same pipeline set and it works. (We also have a pipeline where it partially worked, but here we are not fully aware what pipeline-configs have been applied)

Any ideas?

concourse-bot commented 8 years ago

Hi there!

We use Pivotal Tracker to provide visibility into what our team is working on. A story for this issue has been automatically created.

The current status is as follows:

This comment, as well as the labels on the issue, will be automatically updated as the status in Tracker changes.

friegger commented 8 years ago

Further Investigations

We checked our atc.stdout.log file. There was no 'versions-found' log entry for the partially affected pipeline for commit 1ff876.

vito commented 8 years ago

Make sure you don't have jobs that are doing a put to the same resource. Versions are detected by both check (which respects ignore_paths) and put (which cannot, as it's just pushing a commit, and has to report which commit it produced). See Mechanics of a Pipeline for a more thorough explanation.

If that is what you're doing, I'd declare a separate resource for the puts, with the same config (though that one can omit the ignore paths/etc.). So you'd have one resource for the inbound commits (read-only) and a second one for mutating the repository.

loewenstein commented 8 years ago

How would we keep both resources in sync?

One of the puts we do is promote-candidate in the bosh-openstack-cpi pipeline. Of course we only want to release and promote commits that actually passed the pipeline, i.e. that came from the get resource.

Our use case here is, that we have an auto trigger on the resource to trigger the pipeline and want to avoid that this happens due to the potential commit at then end of pipeline.

Using two separate resources would require a sha1 comparison of both local git repo HEADs inside the task.sh, wouldn't it?

vito commented 8 years ago

I'm not sure what you mean by in sync. You would have one resource that feeds into the start, and one that you use at the very end for promotion, which shouldn't re-trigger your pipeline. You don't want them in sync, otherwise you'd have a feedback loop.

concourse-bot commented 8 years ago

Hello again!

All stories related to this issue have been accepted, so I'm going to automatically close this issue.

At the time of writing, the following stories have been accepted:

If you feel there is still more to be done, or if you have any questions, leave a comment and we'll reopen if necessary!

bruce-ricard commented 4 years ago

@vito We just encountered this issue today: we tried to put our resource that we just pulled, but since it had an ignored path, we got this error when trying to put: ! [rejected] HEAD -> our_branch (fetch first)

Which is not a great error message to help the user understand what is going on. It took us a couple of minutes looking at our resource to understand what was preventing the newest commits to appears. We were thinking that we might want to programmatically prevent anyone from putting a git resource which has a non-empty ignore_paths parameter, and show a good error message. Off the top of your head, do you think it's possible to do that? If so, we are happy to submit a PR.

@angelachin and I from the Pivotal CF Networking program.

vito commented 4 years ago

@bruce-ricard I don't think that fix would be technically correct, though I can see where you're coming from. The problem is that the mistake is actually made in the get step, but the put step can't really know that.

That is, someone could just as easily make the mistake of using a resource with ignore_paths as an input to a put step that's configured without ignore_paths and then hit the same error:

plan:
- get: my-ignored-paths-repo
- put: my-no-ignored-paths-repo
  params: {repository: my-ignored-paths-repo}

The put step doesn't know that my-ignored-paths-repo is a resource configured with ignore_paths; it's literally just a checkout of the repository. We could perhaps modify in to leave a "marker" file or something under .git/ so the put step can check for it but that feels a little bit hacky.

This is actually related to https://github.com/concourse/rfcs/issues/11, for what it's worth - my long-term goal is for Concourse to still collect and use these versions, but know not to trigger builds for them on their own. I feel like that's actually the desired behavior every time people use paths/ignore_paths/ci-skip.

As a workaround, you could consider setting rebase: true.