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

[FIXED JENKINS-6746] Updated to add setting of SVN_MAX_REVISION env #72

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

srice01
Copy link

@srice01 srice01 commented Mar 28, 2014

As per JENKINS-6746, the revision returned in SVN_REVISION is the revision from trunk but not the "current revision". This change sets a new "SVN_MAX_REVISION" environmental variable to be the value of the "current revision" (a maximum of any revisions involved in the repository including any externals). This allows, for example, the setting of the build number to be the current revision so that it is unique if externals, for example, are updated but not trunk.

One thing to note is that if you have multiple SVN locations (different checkouts in the project) then as there is no way to differentiate in "revisions.txt" between these locations so this variable is set to the maximum revision present across all checked-out SVN repositories.

@cloudbees-pull-request-builder

plugins » subversion-plugin #331 SUCCESS
This pull request looks good

@jenkinsadmin
Copy link
Member

Thank you for a pull request! Please check this document for how the Jenkins project handles pull requests

@daniel-beck
Copy link
Member

This doesn't appear to resolve JENKINS-6746 at all.

@srice01
Copy link
Author

srice01 commented Mar 28, 2014

As indicated, it provides what is, effectively, the "current revision" for the repository in an environment variable that can then be used. This provides an alternative to the revision number previously provided by the plugin.

@srice01
Copy link
Author

srice01 commented Mar 28, 2014

Further, to clarify: When an SVN update occurs triggering a build if the change is to the main SVN URL for the job then SVN_REVISION contains the current version so that is fine but if you have externals present in the repository SVN_REVISION will contain the version of the main SVN URL which is not correct (since it's revision number has not changed in the repository). SVN_MAX_REVISION, however, will contain the current version that triggered the build. Effectively, this variable is set to the "current version" for the job.

@daniel-beck
Copy link
Member

JENKINS-6746 refers to labels/output on the UI that confuse the last changed revision and the current revision, rather than environment variables (the comment is by a different author). This change reuses the same last changed revision values otherwise used in the plugin, so it's still different from the revision. So this change does not actually solve JENKINS-6746.

Your original PR comment also seems to indicate that you're not interested in the actual checked out revision (because that wouldn't be provided with this change anyway!), but want to get the highest revision for intra-repo externals without explicitly specified revision, maybe for better reproducibility, synching between different projects, or something similar.

Additionally, this change does not consider externals to different repos, or multiple configured module locations, resulting in useless values in a lot of setups where this simple approach won't be sufficient.

Because of these issues, this change appears to only apply to a very narrow problem you have in your projects, rather than provide a widely applicable solution. It also appears to be difficult to later generalize this to be more flexible in a backwards compatible way.


As an alternative, general approach, you could extend the existing SVN_URL[_X] and SVN_REVISION[_X] to cover not just explicitly configured locations, but all locations participating in the build, including externals. For externals, only those revisions entries that weren't used for SVN_URL[_X]/SVN_REVISION[_X] because they have no corresponding configured module location. Name these new ones SVN_EXTERNAL_URL[_X] and SVN_EXTERNAL_REVISION[_X] to distinguish them from the existing ones and to prevent existing jobs that use hardcoded indices from breaking upon plugin update. Then, in your build script, you could look for the the environment variable with the highest revision value (and possibly your repository as prefix) and just use that.

@srice01
Copy link
Author

srice01 commented Mar 28, 2014

Thank you for your comments. The problem with the general approach is that we do not know how many externals may be present in the project (there may be dozens) and we don't care as we just want the latest or "current" revision so that we can label the build correctly (using the environment variable). So, if we were to use "SVN_EXTERNAL_REVISION_1" whose to say this value is more recent (greater) than "SVN_EXTERNAL_REVISION_2"? This is why I have a single variable "SVN_MAX_REVISION" so that it sifts though all of the URLs and returns a single revision (the maximum) that can be used as the label.

The fundamental issue is that the existing environment variables only return the SVN revision of the main job SVN URL and not any of the other repositories that may be involved (via externals).

If the plug-in were to actually return the actual current revision of the repo that would fix all of this but, as indicated, it does not. It only returns the revision of the main SVN (checkout) URL.

@daniel-beck
Copy link
Member

So, if we were to use "SVN_EXTERNAL_REVISION_1" whose to say this value is more recent (greater) than "SVN_EXTERNAL_REVISION_2"?

This logic should be part of your build script (I do something similar in my Gradle scripts and it's trivial to do). IMO, it just makes no sense in the SVN plugin, at least in the current form, due to the limitations I mentioned.

@srice01
Copy link
Author

srice01 commented Mar 28, 2014

I am not sure which specific limitations you are referring to but looking at the source code it appears that the current revision (for a given SVN repo) is simply not available (only the last changed revision). Is that the limitation you are referring to? The code that I wrote is an approximation of the current revision for the repo (it should in most cases be the current revision since when the build is executing it will have been, most of the time, triggered by an SVN change so therefore the current revision number has to be from one of the URLs relevant for the job - otherwise the build would not have triggered - and it will have to, therefore, be the maximum of these revisions).

I am not interested in the maximum revision specifically, I am looking for the current revision (it is just the maximum revision happens to provide it, or a reasonable approximation, for most cases).

@daniel-beck
Copy link
Member

The limitations I was referring to are mentioned in the third paragraph of my second comment:

Additionally, this change does not consider externals to different repos, or multiple configured module locations, resulting in useless values in a lot of setups where this simple approach won't be sufficient.

This solution does not integrate well with how the plugin works so far and is only narrowly applicable, and it'd be much better if those values were provided more generally -- which my proposal would achieve, at the cost of slightly more work in the actual build script.

(it should in most cases be the current revision).

Not at all. This depends a lot on your repo layout. Especially if you use your repository for multiple projects, those revisions could be very different.

It is, however, the latest revision relevant to the build if considering externals. But only if you don't have externals pointing to other repos, or check out multiple repos -- again, these seriously limit the usefulness of this PR.

@srice01
Copy link
Author

srice01 commented Mar 31, 2014

Your point about multiple repos is perfectly valid. Obviously the code could be slightly adjusted to attempt to determine the maximum per repo (an assumption might be that by "repo" we are referring to the same host which also may be incorrect).

So, if we want to do this correctly to handle multiple repos we will need the current revision for each repo. Is this possible? I believe I had read somewhere that "svnkit" did not provide current revision numbers and, more fundamentally, is it possible to figure out the list of repos?

You will note that I called the variable "SVN_MAX_REVISION" so it does not pretend to be the current revision - It provides a variable that I believe will be useful to people configuring their projects as right now the variables provided by the plug-in are not acceptable (as they do not return the revision of an external if it changes, for example, they always return the revision of the [master] checkout). At least this gives a way of uniquely identifying builds using the revision.

@t-stamm
Copy link

t-stamm commented Oct 9, 2015

Any news on this one?

@recena
Copy link
Contributor

recena commented Oct 9, 2015

@t-stamm I hope to back here soon.

@jglick
Copy link
Member

jglick commented Jun 16, 2017

Same as #105?

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.

7 participants