-
Bug
-
Resolution: Unresolved
-
Trivial
-
None
-
Win XP Pro SP2, Hudson 1.341, Perforce Plugin 1.0.16
-
Powered by SuggestiMate
The Perforce plugin now seems to append the "tail" of the depot path to the client root when auto-generating a client specification. Example:
Client: foobar_cs
View: //depot/projects/foobar/main/...
When syncing, the following client spec view is generated (seen from build console):
//depot/projects/foobar/main/... //foobar_cs/projects/foobar/main/...
Previously (pre-1.0.16) the following was generated:
//depot/projects/foobar/main/... //foobar_cs/...
This behaviour causes all files within the project to end up under e.g. "C:\hudson-inst\jobs\foobar\workspace\projects\foobar\main".
[JENKINS-5343] Incorrect client spec generated
I guess I have to agree with the consistency issue; it might not be such a good idea to have the special case for one line views. On the other hand I find the current implementation to be very non-intuitive. Perhaps it would be best to always require a complete view spec - at least it would then be easy to understand.
Code changed in hudson
User: : rpetti
Path:
trunk/hudson/plugins/perforce/src/main/java/hudson/plugins/perforce/PerforceSCM.java
http://jenkins-ci.org/commit/27974
Log:
[FIXED JENKINS-5343] change form validation to recognize depot-only view lines as being invalid
Ack, I wasn't watching this bug or I would have chimed in earlier. I made the orignal change to support the direct mapping using the simpler left-hand only syntax. I would really appreciate it if you reverted http://jenkins-ci.org/commit/27974 . Let me explain my reasoning:
I've been using Perforce for over 5 years now, first at Google and now at Netflix, so I have seen a lot of different use-cases and created build systems used by thousands of engineers.
The way that we have used Perforce in both companies, and I believe it is common in Perforce shops, is to almost always straight-map of depot paths to local paths. Occasionally subdirs will be mapped around a litte, but in general it is much cleaner to not have to know special mappings to make your projects build. Hiding that knowledge in client specs makes it hard to discover and reproduce. Why would you want your depot and workspace trees to have a different shape?
It is also common that your project will have dependencies on other tools and projects from the depot. Those will often have different root paths. It's not feasible in a large code-base to simply include the top of the depot or to have a single common root for all of these.
So it is necessary to specify the set of project and tool roots. For us, this often looks something like:
//depot/apps/myproject/main/...
//depot/libraries/...
//depot/otherlibraries/...
//depot/tools/build/...
//depot/tools/ant/...
//depot/tools/findbugs/...
And it needs to be mapped straight across. I believe that this is a very common use case, perhaps the most common, which is why I made the default for the RHS be a straight-across mapping to simplify entering these views. We have hundreds of jobs configured just this way.
I also don't believe that prior to 1.0.16 the default mapping was to flatten paths to a single dir. I don't believe there was any support at all for omitting the RHS, and I added what it does now to provide a shortcut for what I believe is the common case.
To me the only reasonable default for the RHS is to make a simple identity map of the LHS and substitute the client name for the depot. Anything else requires special-case knowledge of someones preferred mapping style. Would you take all of the contents of the above trees and automatically dump them into a single directory? Anything other than a straight map needs human input.
I thought I explained it OK when I added the help doc: " ... And, the second member of any pair may be ommitted which will result in an identity mapping being used."
But maybe "identity mapping" is not clear enough.
If someone wants to map //depot/apps/myproject/main/... to //workspace/... then that is a very easy to type single line. When you have many lines in your mapping, having to specify a redundant RHS is tedious and makes the views cluttered and hard to read.
Only the form validation was changed; the functionality still remains intact so as not to break existing jobs. Originally (1.0.13 and earlier) the "view" configuration option mapped directly to the workspace root. I'm not sure when the RHS generation was changed to what it is now, but it was some time after multiple view line capability was added (1.0.14). People upgrading from earlier versions are running into the issue where their files are not being synced where they originally were.
I don't have any problem with reverting the change, but if that's the case then I would like to change the functionality a bit:
The way it works right now makes the assumption that there will only ever be one depot. Of course, having one depot or multiple depots is a matter of preference, but the plugin should still support both cases, especially when many will choose to have multiple depots in order to improve performance. http://kb.perforce.com/AdminTasks/BasicServerConfiguration/SettingUpMul..sInPerforce
Thus, I would like to change the default RHS mapping to be //workspace/depot/etc/etc... rather than //workspace/etc/etc...
Let me know what you think.
Code changed in hudson
User: : rpetti
Path:
trunk/hudson/plugins/perforce/src/main/java/hudson/plugins/perforce/PerforceSCM.java
trunk/hudson/plugins/perforce/src/main/webapp/help/project.html
http://jenkins-ci.org/commit/28042
Log:
JENKINS-5343 reverting change made to view spec form validation in r27974, and adding a somewhat clearer description of what the RHS mapping is set to when it is ommitted.
Ah, I never used Hudson before last June and the 1.0.14 era of the Perforce plugin.
Regarding the breaking of pre-1.0.14 users. Is there a way to know when we are reading a project from an old version? We could automatically expand the old single LHS entry into a single line pair which represents the functionality that use to be implied. That way we support pre-1.0.14 users, and still support the new functionality that I added in 1.0.16 (i think).
Your point regarding the multiple depot case is a good one. But, in my experience I haven't seen an environment where there are a lot of depots, and I haven't yet seen a case where individual projects were split across depots. I suppose it could happen, and we shouldn't preclude it, but I don't think we need to optimize for it.
So, I think adding the depot into the workspace path will be unnecessary most of the time. It will also break all of our Hudson jobs, and anyone else relying on the changes since 1.0.16. I would no longer be able to take advantage of this feature and would have to go and modify all of our jobs to specify the RHS. Maybe we could make the depot name inclusion a checkbox option?
Another idea I had to handle different styles of project layout and RHS generation is to simply have 3 radio buttons to let the user select.
Generate Workspace paths from:
o Flattened depot paths
o Straight depot paths
o Straight paths with depot name
Or, something like that but with better wording
I suggest moving this discussion to the user's list to allow more Perforce users to chime in. Some comments though:
I agree that straight mapping, between the a specific project in the depot and the workspace, is the preferred way to go. I mentioned the possibility of mapping dependencies into separate subdirectories of the main project using the client spec, not because I would recommend that, but because it is virtually the same thing as the "auto-mapping" currently employed + that it IMVHO makes it easier to identify them as dependencies of the main project. Having them at an equal level in the directory tree suggests that they are of equal importance. But that's just a matter of preference I guess.
Generally though, mapping dependencies using a client spec looks a bit brittle to me; anything in the "main" project's dependencies can change under your feet at each sync, causing incompatibility either implementation-wise or interface-wise. Our preferred approach is to branch the dependencies at known versions into a specific subdirectory of the main project and then keep the view/workspace mapping to a single line.
The radio buttons idea sounds good to me. I'll take a look at implementing it this weekend if I'm not doing anything.
As a side note, my company has 28 separate depots in our perforce server, so that's why I brought up the depot names issue.
Changing priority to trivial, since there is a simple solution.