workspace file changes and source control

[raybert]

Hi,

I've set-up CB for my development team for cross platform development (Linux and Windows) and I've noticed some issues regarding changes to workspace files which cause (IMO) unnecessary source control commits.  I'm wondering if I should report this as a bug or change request or if there's something I'm missing to help control this.

The first issue is regarding the path separator characters.  If the workspace file is written on Linux it uses forward slashes and if written on Windows it uses backslashes.  This causes source control (Mercurial, in this case) to mark the file as changed and this then has to be dealt with by the developer when committing changes.  I really don't want tons of unnecessary commits of workspace files just because the slashes are different.  I see this as a bug or design flaw: CB should really standardize on one slash character for its project files and translate them internally as needed.

The second issue is regarding the active project, which is apparently recorded in the workspace file.  This means that if a developer activates a project and the workspace file later gets saved (which usually does end-up happening, CB even nags you to save it) the workspace file is then considered changed by source control and the same issue as above ensues.  We need the workspace file under source control because it records vital project dependencies.  It seems to me though that something such as which project is active should be stored in a different file -- one which is not intended to be under source control (a layout file, perhaps?) -- in order to avoid this issue.

I'd appreciate any input that folks may have on this.

Thanks,

~ray

[MortenMacFly]

I'd appreciate any input that folks may have on this.

Well -  the workspace is modified if the user saved it by choosing the appropriate "Save" command in C::B or answering "Yes" if C::B asks to save the modified workspace. In that case how to differ whether a dev was just "lazy" or really wanted to save something? Depending on the workspace setting a different active target might be vital for development (that's why it's in the workspace file). So how to differ again if a dev did it on purpose or not?

I'll propose another idea: I don't know mercurial but SVN offers pre-commit checks. These can call a pre-commit tool before the actual difference is computed. Knowing that a workspace file is just XML it should be an easy task to setup a tool that takes care of e.g. "fixing" the back-slashes. The rest I don't know how to properly handle. IMHO it's up to the dev to decide what (s)he wants to save (thus to commit).

[killerbot]

I can understand the OP, we have the same issue with the version control system ClearCase.

I do think, CB should use in the project files / workspace files either forward slashes or backward slashes. I would suggest forward slashes (event without a version control when you want to compare sources between windows / linux these / \ differences are annoying).
That CB can handle them both is good, for it's multi platform support, but the xml files are for CB. And either work on both operating systems, I know, since I mix them a lot, and as long as you don't (re)save your project, nothing changes.

And I don't think we should expect users to whatever pre-commit or create filters whatsoever. This is just not user friendly, it's our job to do better ;-)
Then it will be solved for everyone, because a user shouldn't bother how CB stores it's information internally.

Note that CB's xml files are easy to understand, which can not be said for other IDE's (some of them put their version number in the project file, something like 9.0, but depending on the locale that can become 9,0, guess which company made such a stupid thing  :mrgreen: )

As for the workspace, one can work around it by not saving it when CB asks you to do so. Counter intuitive maybe.
But there a e few other things wrong with the workspaces, maybe all of them one day could be tackled.

[MortenMacFly]

This is just not user friendly, it's our job to do better ;-)

Depends on the view. I personally think that if a filename is provided in a workspace file (which is) it should be a OS native filename. Under Windows a path containing forward slashes is not native and does not even function. The only solution I see is to use a protocol like file:// or alike. This would again be "native" and correct on all platforms.

[MortenMacFly]

Forward slash path separators in source code work on all platforms because the compilers are required by market forces to accept UNIX source code.

In case you did not realise: We are not talking about source code here, but a workspace file. Better read twice before "Bzzting"... ;-)

[killerbot]

calm calm.

Everybody is overlooking something, the workspace file, the project file are internal CB files !!!
And therefor I *think* forward/backward slashes will work whatever you use, and wherever you use it.

I use backslashed cbp files on linux, and forwardslashed cbp files on windows, an I haven't encountered any issues.
This is most probably because CB those some translations to native on the inside.

That's why I think, that for example just using forward slashes will not introduce any issues.
Always something we can try out ...

[raybert]

Hi guys,

Thanks for the comments.  Some follow-ups:

WRT slashes in the workspace file, this is a CB file exclusively so there is absolutely no need for the paths contained within to match the native file system.  CB can already handle either slash on both input and output.  The real issue here is about CB itself being consistent.  Why should it use two different formats?  That's inconsistent.  It should use one everywhere.

If you have a cross-platform workspace then the slashes used in the workspace file are always going to be "wrong" on one platform or the other.  If you save it on Windows you'll have backslashes when you return to Linux and vice-versa.  So there's really no meaning to matching the native file system in the first place.

It's clear to me it should use only one, consistently.  The file:// URL suggestion also seems reasonable, if one wants to be really anal about this, but I personally think that doing that is overkill.  There's really nothing wrong with always using forward slashes (for example).

As for the active project issue, I'm not sure I agree that this information is important to the project configuration (it certainly isn't useful to me).  If you can cite an example of when this might be important I might change my opinion.  In most cases, programmers are going to have to purposely change the active project while they're working for one reason or another.  They should understand what each project is and know which one they need at any given time.  Also, each project should build independently, so this is not an issue of the build failing if you don't have the "correct" project activated (maybe you won't get the build you wanted, but a programmer that can't handle that is not someone I want touching my code!).

I therefore consider the active project setting to effectively be a project-specific *user preference*.  I think we can agree that user preferences should be stored separately from project configuration.

The suggestion of using a VCS commit hook (yes, Mercurial does have them, BTW) is not a bad idea as a work-around.  But it's important to understand that that's all it is: a work-around.  CB shouldn't make work-arounds necessary as a matter of course.

BTW, I'm not sure if it's possible to write a VCS hook that *modifies* the file being committed because the changes would have to be propagated back to the person doing the commit.  If you can't do that then the person's working copy is immediately dirty after a commit (WRT to the central repository) and that's going to really goof people up.  You would have to reject the commit altogether, I think.  And that means that developers are going to be forced to omit the workspace file from their commit and/or revert the changes to it.  And they're going to have to do this *all the time*.  I don't think it's reasonable to force that on folks.

As for the suggestion of relying on developers to NOT save the workspace file, um, yeah, right.    There are all different levels of developers and even the best are going to forget occasionally.  I personally like to use "save everything" when I have changes rather than deal with multiple nag prompts to save things.  If I have to avoid saving the workspace file then I effectively can no longer do that and I have to live with all the nags.  Also, isn't it possible to turn the nag prompts off?  If I remember that correctly, and someone does turn them off, they're going to be saving the workspace file all the time without even realizing it!

Thanks again,

~ray

[MortenMacFly]

WRT slashes in the workspace file, this is a CB file exclusively

Well - I am open for everything. Just keep two things in mind:
- There are already converter around that convert C::B projects (workspaces) to different IDE's. So we break the current format as we introduce a new version (we did it already in the past...).
- Looking at other cross platform IDE's and how they handle file pointers still makes me believe that if we do a change it should be something like a file:// protocol which is "native".

[raybert]

Well - I am open for everything. Just keep two things in mind:
- There are already converter around that convert C::B projects (workspaces) to different IDE's. So we break the current format as we introduce a new version (we did it already in the past...).

These changes shouldn't break any third-party utils though:

- Any such tools already have to deal with both slashes (because they already differ depending on which OS you saved the workspace under).

- The "active" attribute is optional on any given "Project" tag so no tool should require it to exist.

- Looking at other cross platform IDE's and how they handle file pointers still makes me believe that if we do a change it should be something like a file:// protocol which is "native".

I certainly would not mind that.  My only goal is to make the content more consistent and avoid unnecessary changes.

~ray

[raybert]

Just thought of something: if you use file:// URLs then that WILL break third party tools.  So this seems to be a good argument for sticking with simple directory paths with one type of slash (forward, preferably).

~ray

[MortenMacFly]

- Any such tools already have to deal with both slashes (because they already differ depending on which OS you saved the workspace under).

Exactly not. That's my point: The tools just "copy" the content as a file string. This then is used to open the file pointer using (probably) OS calls. So if we change the path forward slashes on Windows such functionality won't work anymore.

Anyways - I think the topic has been discussed enough. Time to get hands on and provide patches...

[raybert]

hmm... so, if I save a workspace on Linux, commit it to source control, refresh my Windows work area, and then try to convert it using one of these tools, it will fail because the tool doesn't bother to check the slashes?

If that's the case then I would argue that the tool in question has a bug.

As for patches, I was thinking last night that if I patch CB and make my team use the patched version then that's one way forward for me (and it's cleaner than using VCS hooks).  I imagine that these patches wouldn't be very difficult.  So I will likely try to find some time to work on this and I'll submit my patches (even if the devs ultimately decide not to commit them).

~ray

[MortenMacFly]

So I will likely try to find some time to work on this and I'll submit my patches (even if the devs ultimately decide not to commit them).

Sounds good to me. Notice that we do discuss patches internally. So it *is* likely it will be applied as at least one other dev pro's the modification... ;-) That I don't like it doesn't necessarily mean a show-stopper.

[raybert]

I finally got around to working on a patch to address these issues.

The attached patch (against svn 5456) causes filenames in workspaces to be always written with forward slashes.  It also disables saving the active project.  Additionally, it prevents the workspace from being marked dirty when the active project is changed (doesn't make much sense to do that if it's not being saved anyway).

One thing it doesn't do is to save the active project in a different file as I suggested earlier.  I don't need that for my own use but, if it's desired, it shouldn't be hard to do (and I'll try to find time to do it).

This is my first patch and I'm not sure of the protocol here.  Should I post this to the tracker as well?  I have a few more itches to scratch so this probably won't be my last.

~ray


[attachment deleted by admin]

[killerbot]

yes, please add if through the tracker on our berlios page, so it won't get lost ;-)

[MortenMacFly]

I finally got around to working on a patch to address these issues.

Two notes:
1.) I don't get why you do this:

Code

-        if (m_pWorkspace)
-            m_pWorkspace->SetModified(true);
+//        if (m_pWorkspace)
+//            m_pWorkspace->SetModified(true);

This breaks C::B's ability to detect a change in the workspace.

2.) Are you aware that wxFileName has it's own Windows/Unix convertion of path names? Check the wxPathFormat enum (see http://docs.wxwidgets.org/2.8/wx_wxfilename.html) for that purpose. This way you don't need the ExportFilename method at all.

Other that that I am surprised that this is such a minor change... But it seems to work though... :shock:

[raybert]

Two notes:
1.) I don't get why you do this:

Code

-        if (m_pWorkspace)
-            m_pWorkspace->SetModified(true);
+//        if (m_pWorkspace)
+//            m_pWorkspace->SetModified(true);

This breaks C::B's ability to detect a change in the workspace.

No, it only avoids marking the workspace dirty when a project is activated (the change is in ProjectManager::SetProject()).  All other workspace changes are still recorded.

2.) Are you aware that wxFileName has it's own Windows/Unix convertion of path names? Check the wxPathFormat enum (see http://docs.wxwidgets.org/2.8/wx_wxfilename.html) for that purpose. This way you don't need the ExportFilename method at all.

My goal wasn't to change filename format, only standardize the slashes.  So I used the same file name format that CB would have normally written except I force backslashes to forward slashes before writing.

This works in the workspace file because all of the paths written there are forced to be relative paths (relative to the workspace file) and relative paths look the same on Unix and Windows (notwithstanding the slashes).

~ray

[raybert]

Hi,

I've significantly updated the patch that I previously posted.  It's a much more complete solution now.  However, I included a couple of (minor?) additional behavioral changes that I'd like to hear feedback on.  I'll also post this to the tracker shortly.  Note that this new patch contains all the changes from the previous version.

The new changes are:

• Now creates a "workspace layout" file that records the active project and something new called the "preferred target" (more in a moment).  In order to avoid overwriting project layout files (in the case where the workspace and a project have the same name) I've used the filename extension ".workspace.layout".  So a workspace file called "foo.workspace" will now have a companion layout file called "foo.workspace.layout".  (The intention, of course, is that nothing should be placed in this file that should be in source control, similar to the project layout file.)  I'm not sure that I caught all of the conditions in which the workspace layout file needs to be written; I'd appreciate feedback on that.
• Added the concept of a "preferred target".  I found the previous behavior of changing the user-selected build target whenever a project was activated to be annoying.  I've often found myself building the wrong target because I forgot to check whether CB changed it on me.  Now, when a user selects a build target, it gets set as the "preferred target" and CB will try to maintain it as the selected build target when new projects are activated (this is not possible of course when a given project does not have the preferred target).  It also saves the last preferred target in the workspace layout file so that it will be restored when the workspace is loaded, if possible.  This solution seems to be a reasonable compromise to the problem.
• I modified the target drop-down (and menu) list so that it does not show targets that are not supported on the current platform.  If you have a project that has no supported targets then the list ends-up being empty.  I'm not sure if anything needs to be done about that.

Please let me know what you think of these changes.

Thanks,

~ray


[attachment deleted by admin]

[ironhead]

Sorry for resurrecting such an old thread, but I think there is a lot of merit in having the .workspace.layout file contain the more transient workspace information.  Having the active indicator in the workspace file causes issues with SCM tools due to the fact that any time another project is activated (and the workspace file is saved), the SCM tool will detect the change and prompt for a commit.

Edit: taking it one step further, it would be nice to have .workspace.layout.$(UserID) (or some variation thereof) so each user can have their own transient file.

[MortenMacFly]

any time another project is activated (and the workspace file is saved), the SCM tool will detect the change and prompt for a commit.

Well fr me it's very simple: I simply don't commit that file then. Hence I am already testing this patch for some time now. One thing I don't like is that if you don't have that layout file (for me it's always after creating a clean checkout) not project is activated because there is no fall-back for the case the layout file is not present (which can of course happen and should be considered). Additionally it happened to me that the workspace file was modified but C::B did not tell me (probably because I did not have a layout file?!).

I had in mind to look into that but I am very limited in time atm. So - if you want to help, try the patch and help fixing the bugs I mentioned.

BTW: Please refer to patch #002748 for that purpose, not the one in this forum.

Edit: taking it one step further, it would be nice to have .workspace.layout.$(UserID) (or some variation thereof) so each user can have their own transient file.

That means you are working on the same code base (file base) with several developers? Why do you need version control then? I don't see the benefits from doing this. Because any layout file should not be under revision control. Thus everybody that creates an own workspace will ultimately have it's own layout file.

[Seronis]

That means you are working on the same code base (file base) with several developers? Why do you need version control then? I don't see the benefits from doing this. Because any layout file should not be under revision control. Thus everybody that creates an own workspace will ultimately have it's own layout file.

Wouldnt this be useful for the instance where you have multiple developers and a few of them happen to work on the project from multiple different locations?

[MortenMacFly]

Wouldnt this be useful for the instance where you have multiple developers and a few of them happen to work on the project from multiple different locations?

What I meant was: If you do use version control why would you still allow several devs to work in one (file) place? I mean: version control is exactly the tool needed to avoid conflicts that will surely arise if several people work on the same file base.

Thus the best way is everybody having it's own working copy which renders the USER prefix useless... just my 2 cents.

[ironhead]

Thus the best way is everybody having it's own working copy which renders the USER prefix useless... just my 2 cents.

Fair enough, I do agree with you here.

Regarding:

Well fr me it's very simple: I simply don't commit that file then. Hence I am already testing this patch for some time now. One thing I don't like is that if you don't have that layout file (for me it's always after creating a clean checkout) not project is activated because there is no fall-back for the case the layout file is not present (which can of course happen and should be considered). Additionally it happened to me that the workspace file was modified but C::B did not tell me (probably because I did not have a layout file?!).

Again, I'm inclined to agree with you, in that I would normally not commit a workspace file to SCM.  The challenge is that project dependencies are stored in the workspace file (i.e. Project X depends on Project Y) so without committing the workspace file, someone checking out the source may have build failures due to projects being built in the wrong order.

I will try out this patch hopefully next week (I have to admit I haven't actually tried it yet, I was just going based on the description).

[killerbot]

Yes, the fact that project dependencies re in the workspace, is sometimes a game spoiler.

What would the cons for putting those dependencies in the project file ?

[oBFusCATed]

I vote 100% for putting deps in the project file.
At work we have around 60 project 5-10 solutions, half of the projects are libs.
It is great pain to create a new application that depends on several libs.
We are using VS 2005 .
It costs lots of time, that can be save with that feature. Let us beat the M$!
Also it could be possible to automatically load the dependant projects.

[nanyu]

Looking forward this function come with nightly builds.

[MortenMacFly]

Again, I'm inclined to agree with you, in that I would normally not commit a workspace file to SCM.

YOu misunderstood. Surely I am comitting the WS file to SCM. But only at times where I setup the WS or modify it (moving projects, adding / removing projects, changinfg dependencies). What I meant is that I will not commit the WS file if only the active project is changed and this makes no sense for other devs, too.

However, maybe that is because we have a QA process setup that is before every commit. So we not just commit, but review before. Thus such a modification easily gets spotted and thus can be ignored.

[oBFusCATed]

Morten: Is there any chance, this patch can get into trunk?

At the moment I'm working cross platform, I've modified my project and now the whole project should be committed
So, with the current version I get no chance to see what I've changed in a particular revision.

[MortenMacFly]

Morten: Is there any chance, this patch can get into trunk?

Well in fact I've maintained this patch during the time and also using it.

The reason why I am not committing is the following serious drawback:

Well you have all your changes in the layout file now, but also the pointer to which project is the active one.

If you do commit the layout file you have the same thing happen: This will change often.

If you do not commit the layout file you are missing to point to the active project so C::B would set the first one as active which will break your compilation most likely if you do a clean checkout (take project dependencies into consideration). Also in this case I believe the active project in a mandatory information as it controls the build process and therefore should really remain in the workspace file or somehow provided to the developer. Probably a "default active project" can remain in the workspace and a "current active project" could be in the layout. But then: When and how to update the "default" one?!

I don't have a good idea how to resolve this so far... if someone has - step forward.

For the moment - always using forward slashes (the unix way) in the project and workspace files (so partially applying this patch) might be something better.

[oBFusCATed]

For the moment - always using forward slashes (the unix way) in the project and workspace files (so partially applying this patch) might be something better.

If you can apply this part of the patch I'll be 99% happy, the active project is manageable (you can easily revert the change before committing), but the slashes are not

[oBFusCATed]

Ping ping pinggggg....
Patch please, really please, please...