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

Perforce VCS Plugin #55

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

Conversation

prat0088
Copy link
Contributor

I tested interactively on Linux multiple times with --initialize, push, pull, and adding versioned files. I'm not yet using this in production because of unrelated reasons, and it might be another month or two.

I haven't tested on Windows but believe it will work. The only slightly OS-specific thing I'm doing is piping a file to STDIN using 'cmd < file', which I understand to work on Windows.

Relates to discussion over at #53 .

@prat0088
Copy link
Contributor Author

Where should I add documentation? I see some pod under doc, but it doesn't appear to contain all of the documentation for VCS plugins that is posted to serge's public site.

@codecov-io
Copy link

codecov-io commented Dec 30, 2016

Codecov Report

Merging #55 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master      #55   +/-   ##
=======================================
  Coverage   67.37%   67.37%           
=======================================
  Files          49       49           
  Lines        4383     4383           
  Branches     1161     1161           
=======================================
  Hits         2953     2953           
  Misses        802      802           
  Partials      628      628

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4625afa...f6e6606. Read the comment docs.

@iafan
Copy link
Contributor

iafan commented Jan 3, 2017

Where should I add documentation? I see some pod under doc, but it doesn't appear to contain all of the documentation for VCS plugins that is posted to serge's public site.

Plugins historically didn't have any documentation in Serge repo — the documentation is maintained in a separate (private) repo for serge.io website. Can you provide your documentation as an embedded POD in the ,pm module itself? I'll use it as a base and will create a page on Serge.io website.

@iafan
Copy link
Contributor

iafan commented Jan 3, 2017

@prat0088 I know you tried to approach me on IRC the other day about Perforce concepts, but I wasn't available at the time. Can you ping me later today? I'd like to ask some questions and make sure I fully understand this, since I have to way to test Perforce.

Speaking of configuring repos, I believe we could stick with the already notion of URLs, since Perforce has them, but indirectly. Consider the initial cloning command from their documentation:

p4 -u bruno -d Ace clone -p perforce:1666 -f //depot/main/...

This means that you want to connect to perforce:1666 server and clone remote //depot/main/ directory into a local clone one. So we could construct the "URL" string like this: perforce:1666//depot/main/

If Perforce server is something that can be (and usually is) omitted, the "URL" can simply become //depot/main. See also how this is done in git-p4 — they seem to only work with one server, as they only specify a remote path.

Since Serge can handle multiple remote repos per config, it allows you to pick relevant Perforce folders to form local file structure. So Serge config might look like this:

sync
{
    vcs
    {
        plugin                   p4

        data
        {
            local_path           /var/serge/data/my_project

            remote_path
            {
                main             //depot_main/app/path/to/i18n/
                widget           //depot_widget/app/path/to/i18n/
                # ... and so on ...
            }
        }
    }

}

We could also add a remote_server parameter to specify the remote server once (which can be overwritten at each path level):

sync
{
    vcs
    {
        plugin                   p4

        data
        {
            local_path           /var/serge/data/my_project
            remote_server        another_server:1666

            remote_path
            {
                main             //depot_main/app/path/to/i18n/
                widget           yet_another_server:1666//depot_widget/app/path/to/i18n/
                # ... and so on ...
            }
        }
    }

}

@prat0088
Copy link
Contributor Author

Sorry for the late reply. I was out for vacation. I'll reply to these comments and the others over the next few days. I'll also try to find you on IRC. Probably late afternoon / early evening PST.

@prat0088
Copy link
Contributor Author

prat0088 commented Jan 16, 2017

I rambled a lot about what to do with paths and URLs while trying to work through a few interesting situations with how we've configured Perforce here, but I ended up about at your comment. That sounds good.

I'll update the documentation and clean up a few bits to bring it closer in line with your comment.

@prat0088
Copy link
Contributor Author

Per-branch servers strikes me as a very rare case (ie. somehost.com:1666//depot/blah). I'm under the impression organizations have a single server that is used to store everything. In my opinion it's unneeded.

@prat0088
Copy link
Contributor Author

I'd like to draw your attention to

my $SERGE_P4RC_FILENAME = '.p4rc_serge';

I save the client root in this file only for get_remote_url(). I can make it slightly nicer by storing the client name in the file, then getting the depot path by querying p4 with the client name. This would assume a single (eg. //depot/proja/...) which I think we're in agreement on.

I was originally not thrilled about this approach (store client name in .p4rc_serge), but there are two functions:

  1. Making get_remote_url() work
  2. Allowing the user to easily inspect the client name for a directory if they need to troubleshoot something. If the plugin always constructed it dynamically from the config the user would have to search through all of the .serge files for the correct config, find the job that corresponds to the path, and then look up in the documentation to see what rules Serge applies to construct the client name.

Additionally, it's common to have a local .p4rc file in the root of your local directory that stores client name, so it would be somewhat familiar to Perforce users, yet recognizable as being generated by Serge.

I just wanted to run this approach by you.

@iafan
Copy link
Contributor

iafan commented Jan 26, 2017

Per-branch servers strikes me as a very rare case (ie. somehost.com:1666//depot/blah). I'm under the impression organizations have a single server that is used to store everything. In my opinion it's unneeded.

That's fine. Use your judgement. So you want to have a per-config remote_server option and remote paths would just represent paths on the server (e.g. //depot_main/app/path/to/i18n/)?

@iafan
Copy link
Contributor

iafan commented Jan 26, 2017

Making get_remote_url() work

So when you do a p4 clone ... command and have your local checkout folder, this folder by default doesn't have any Perforce-specific metadata file or folder (like .git)? Is .p4rc created manually or by the p4 tool? If that's a common place to store config, can Serge probably create/update this file instead of having its own one?

Having a custom '.p4rc_serge' is also fine, but I just want to make sure we're aware of alternatives and are using the best reasonable approach.

@prat0088
Copy link
Contributor Author

prat0088 commented Jan 27, 2017

So when you do a p4 clone ... command and have your local checkout folder, this folder by default doesn't have any Perforce-specific metadata file or folder (like .git)?

Correct. The first step is

p4 client # Create a client that maps your local directory to the server and contains other settings like the local config in .git. Except that this lives on the server and you refer to it by a unique, short name.

This doesn't create any local file information.

Then you would usually add the client name to a local file whose name is specified in P4CONFIG.

Finally you call p4 sync to pull down all of the files.

There isn't a 1-command equivalent of clone.

Is .p4rc created manually or by the p4 tool? If that's a common place to store config, can Serge probably create/update this file instead of having its own one?

It's created manually. There is no standard from Perforce the company as far as I can tell. We use .p4rc. I've seen that on mailing lists. I've also seen p4config.txt on stack overflow.

Now that I think about it, it would make sense if this was also a config setting the user could define. Then Serge could create/update this file. I think that would be a good solution.

@iafan
Copy link
Contributor

iafan commented Feb 1, 2017

Thanks for additional info, Tristan. I have one more question: what can possibly happen if one tries to clone (or do whatever the equivalent of git pull in p4 is) into an existing fodler with some unrelated content? Will p4 complain? Will it simply delete bogus data?

The reason I ask is at the very basic level the p4 sync plugin could simply ignore the actual remote folder and make get_remote_url() always return some constant string.

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