Jump to content


P4API.NET Issues


  • Please log in to reply
21 replies to this topic

#1 keick

keick

    Member

  • Members
  • PipPip
  • 13 posts

Posted 20 March 2014 - 09:13 PM

I'll be the first one to admit I'm a newb when it comes to Perforce. After much arm twisting, I'm planning on migrating my company from SurroundSCM to Perforce. In order to do this, and to truly evaluate the 'power' of the Perforce API, I decided to embark on a converter written in C#.

I admit I started with the SurroundSCM side, and outside of one small issue, it is the most straight forward API (no offense).

Then I started in the Perforce, with high expectations. Well, ya know what they say about high-expectations...

The number of coding errors an bugs in this thing are astounding. So I'll start with the one that bit me for the longest time (besides using it wrong).

I'll start with changelists. Initially I was expecting to have to create a changelist, add/modify the files that I want in it, then submit. Easy-peasy. I was corrected by my brother, saying I shouldn't create my own changelist but use the default. I have found NO way of doing that from the API... So on to the bugs.

The worst offender is the misuse of TryParse. Where I go bit first was in FromChangeCmdTaggedOutput(). Here is the code:

if (objectInfo.ContainsKey("Change"))
{
int v = -1;
int.TryParse(objectInfo["Change"], out v);
Id = v;
}
else if (objectInfo.ContainsKey("change"))
{
int v = -1;
int.TryParse(objectInfo["change"], out v);
Id = v;
}

TryParse is an OUT parameter, not a REF. So it is guaranteed to assign a value, and in this case 0. In my case, the server was returning 'new' not a number. This routine then assigned the number 0 which is valid, but already used by the server causing all sort of havok. This is what it should look like, if you really want to return (-1).

if (objectInfo.ContainsKey("Change"))
{
int v;
if (int.TryParse(objectInfo["Change"], out v)) Id = v;
else Id = -1;
}
else if (objectInfo.ContainsKey("change"))
{
int v;
if (int.TryParse(objectInfo["change"], out v)) Id = v;
else Id = -1;
}

I quick glance through the code base I see this error repeated over and over. For example:

DateTime v;
DateTime.TryParse(objectInfo["Date"], out v);
ModifiedDate = v;

What does a date of 0 look like?  Or maybe a 0 file size?

	 if (objectInfo.ContainsKey(sizeKey))
	 {
	 long size = -1;
	 long.TryParse(objectInfo[sizeKey], out size);
	 file.Size = size;
	 }

The above will return 0, not -1, if passed a mal-formed integer.

I wish I could say my converter is coming swimmingly, but between the completely non-.NET design of the API I think I'm better off just using PInvoke to the CPP library. Like, why does the factory function NOT return a valid changelist? How am I supposed to know that it just creates a useless, non-connected, changelist that has to be modified then passed to CreateChangeList before it's usable? Who designed factory functions that don't complete the job?


Changelist cl = rep.NewChangelist();
cl.Description = actions[0].Comment;
cl.OwnerName = actions[0].User;
cl = rep.CreateChangelist(cl);

I'm sorry, but that code is just horrible in so many ways. I have to create an object fill it out, then re-create it before it's useable...

ARGH I need major help here, because something doesn't smell right here; but for the life of me can't figure out what i'm assuming/doing wrong.

#2 P4Matt

P4Matt

    Advanced Member

  • Members
  • PipPipPip
  • 1383 posts

Posted 21 March 2014 - 03:41 PM

I'll get your feedback on the use TryParse() to the dev team; it looks like they may want to revisit that code.

You should be able to just fetch the default changelist from the server; I need to re-build my Windows dev VM but I'll get you some sample code.

#3 keick

keick

    Member

  • Members
  • PipPip
  • 13 posts

Posted 21 March 2014 - 08:54 PM

My brother kept on harping about using the default changelist as well, but for the life of me could not figure out how. A assume the only way to add/edit files is via a changelist.

#4 P4Matt

P4Matt

    Advanced Member

  • Members
  • PipPipPip
  • 1383 posts

Posted 25 March 2014 - 04:49 AM

I wrote what is probably the world's dumbest .net application, but it shows how to get the default changelist:

		static void Main(string[] args)
		{
			// define the server, repository and connection
			Server server = new Server(new ServerAddress("qaplay.perforce.com:1999"));
			mRepository = new Repository(server);
			mPerforceConnection = mRepository.Connection;

			// use the connection varaibles for this connection
			mPerforceConnection.UserName = "matt";
			mPerforceConnection.Client = new Client();
			mPerforceConnection.Client.Name = "Some_client";

			// connect to the server
			mPerforceConnection.Connect(null);

			Changelist c = mRepository.GetChangelist(0, null);
			Console.Write(c.Description);
		}
If you had run any adds or edits prior to running GetChangelist() you'd see them listed in the files section. As long as you set the description on it you should be able to turn around and submit that change.

#5 keick

keick

    Member

  • Members
  • PipPip
  • 13 posts

Posted 26 March 2014 - 05:09 PM

I look forward to trying this out. Since this is a migration, and I want to keep the original dates. I will need to edit the change list submit datetime though, is that possible in the first submit(), or does it specifically have to be a post edited submit?

#6 P4Matt

P4Matt

    Advanced Member

  • Members
  • PipPipPip
  • 1383 posts

Posted 26 March 2014 - 06:37 PM

You'll have to force it post-submit as a super user.

#7 keick

keick

    Member

  • Members
  • PipPip
  • 13 posts

Posted 26 March 2014 - 07:38 PM

Ok, I must be doing something wrong here. The following code wont compile, it's a mash of several other functions. BUT the problem comes back down to the default changelist code.

The problem stems from the fact that the following line (from your example) returns 'new' instead of a valid number. If I step into FromChangeCmdTaggedOutput, the line objectInfo["Change"] has a value of "new". So the original TryParse logic turns that into a 0, the fixed logic turns it into a -1. I've tried it the old and new way, but the gist is that as soon as I call cl.Submit() I get an exception that the number isn't a valid changelist.

Changelist c = mRepository.GetChangelist(0, null);

Here is my flattened out code. The AddFile calls work flawlessly, assuming that they are going to the default changelist here.

//
//Create and connect to server per the command-line args
//
Server server = new Server(new ServerAddress(options.P4Server + ":" + options.P4Port));
repository = new Repository(server);
repository.Connection.UserName = options.P4User;
Options opts = new Options();
opts["Password"] = options.P4Pass;
repository.Connection.Connect(opts);
//
//Create/Open a client named imports with a root directory in our windows temp folder
//
client = repository.GetClient("imports");
if (client == null) {
client = new Client();
client.Root = System.IO.Path.GetTempPath();
client.Name = "imports";
client.Initialize(repository.Connection);
client = repository.CreateClient(client);
}
client.Root = System.IO.Path.GetTempPath();
repository.Connection.Client = repository.UpdateClient(client);
//
//Later on, in another function... Add a new file to the repository, with the new file being
// nested under the windows temp path
//
string clientImportRoot = Path.Combine(client.Root, "importing"); //TBD define import root on commandline
AddFileAction addFile = action as AddFileAction;
if (addFile != null) {
//TODO: Map from action path, to target path
string mappedDepotPath = "";
string mappedClientPath = Path.Combine(clientImportRoot, addFile.Path);
//Get the file from the 'other' repository, and place it into our client path
System.IO.Directory.CreateDirectory(mappedClientPath);
string tempFile = source.GetFileForAction(addFile, mappedClientPath);
FileSpec fs = new FileSpec();
fs.LocalPath = new LocalPath(tempFile);
//This works now :)
client.AddFiles(null, fs);
}
//
//Here I want the default changelist, set the description and owner name... and submit to complete the 'add'
//
Changelist cl = repository.GetChangelist(0, null);
//However, this call ^ returns the text 'new' as the changelist ID; That is not the default changelist!!!
cl.Description = actions[0].Comment;
cl.OwnerName = actions[0].User;
Options op = new Options();
//This generates an exception, (-1) isn't a valid changelist id.
cl.Submit(op);


#8 P4Matt

P4Matt

    Advanced Member

  • Members
  • PipPipPip
  • 1383 posts

Posted 26 March 2014 - 10:41 PM

I think that Submit() method has a bug. Try SubmitFiles() method on Client. Here is some sample code from the upcoming rewritten docs:

// set the options for the p4 submit command
// 0 to specify default pending changelist, null for changelist since we are using the default
string description = "update to test.txt";
// set reopen to false
ClientSubmitOptions clientOptions = new ClientSubmitOptions(false,SubmitType.SubmitUnchanged)
SubmitCmdOptions options = new SubmitCmdOptions(SubmitFilesCmdFlags.None,
				   0, null, description, clientOptions)
// create FileSpec with null for ClientPath, LocalPath, and VersionSpec
string depotPath = "//depot/main/test.txt";
FileSpec fileToSubmit = new FileSpec(new DepotPath(depotPath), null, null, null)

// run the command with the current connection
SubmitResults results= con.Client.SubmitFiles(options, fileToSubmit);


#9 keick

keick

    Member

  • Members
  • PipPip
  • 13 posts

Posted 27 March 2014 - 12:03 AM

I had looked at that, but would prefer to submit multiple files in a change list (converting a rather large SurroundSCM repository). The SubmitFiles() function only takes a singular file despite the name being plural.

I have number changelists working, so I'll stick with that for now. But my next issue revolves around changing a submitted changelist, namely the date modified, and user. I tried different variants on the following theme with nothing but exceptions; Even though the docs say that the -f flag is allowed.

//Submit the files, using no options
cl.Submit(null);
//Change the date and owner
cl.ModifiedDate = actions[0].Stamp;
cl.OwnerName = actions[0].User;
//Update the changelist
Options op = new Options(ChangeCmdFlags.Force);
cl.Submit(op); //Exception occurs
//Alternate update - my own function
repository.UpdateForceChangelist(cl);

I made my own custom function within Repository.Changelist.cs because the function I wanted to use SaveChangeList() was private.

public Changelist UpdateForceChangelist(Changelist change) {
if (change.Id <= 0) {
throw new ArgumentOutOfRangeException("change", "Can only update a numbered changelist");
}
Options op = new Options(ChangeCmdFlags.Force); //allows modification of date and owner
return SaveChangelist(change, op);
}

Alas, this also generates pretty much the same exception as the Submit() did when using the -f.

So using the P4API.NET is there a way to force the read-only user and modified fields?

#10 P4Matt

P4Matt

    Advanced Member

  • Members
  • PipPipPip
  • 1383 posts

Posted 27 March 2014 - 03:14 AM

I'd recommend playing with Perforce a bit from the command line just to get a sense of how it works in general. You can optionally provide a file spec, but it's not required. With Perforce, even if you do provide a file spec to narrow down the set of files, you can use the recursive wildcard '...' to specify all files below a path. Send null to that SubmitFiles() call and you should be golden.

Try this, it works in my test project.

Options options = new Options(ChangeCmdFlags.Force);
Changelist c = mRepository.GetChangelist(252437, null);
c.OwnerName = "matt";
c = mRepository.UpdateSubmittedChangelist(c, options);

Make sure you are running your Perforce commands as at least an admin user.

#11 p4bill

p4bill

    Advanced Member

  • Members
  • PipPipPip
  • 217 posts

Posted 27 March 2014 - 04:10 AM

View Postkeick, on 27 March 2014 - 12:03 AM, said:

I had looked at that, but would prefer to submit multiple files in a change list (converting a rather large SurroundSCM repository). The SubmitFiles() function only takes a singular file despite the name being plural.

SubmitFiles() takes a singular FileSpec. If you create a FileSpec that represents the top level wildcard, you can submit all files in a changelist, including the default. Editing Matt's prior example:

// set the options for the p4 submit command
// 0 to specify default pending changelist, null for changelist since we are using the default
string description = "update to test.txt";
// set reopen to false
ClientSubmitOptions clientOptions = new ClientSubmitOptions(false,SubmitType.SubmitUnchanged)
SubmitCmdOptions options = new SubmitCmdOptions(SubmitFilesCmdFlags.None,
								 0, null, description, clientOptions)
// create FileSpec with null for ClientPath, LocalPath, and VersionSpec
string depotPath = "//...";
FileSpec fileToSubmit = new FileSpec(new DepotPath(depotPath), null, null, null)
// run the command with the current connection
SubmitResults results= con.Client.SubmitFiles(options, fileToSubmit);

will submit all files that are in the default pending changelist.

The wildcard used is:

string depotPath = "//...";


#12 keick

keick

    Member

  • Members
  • PipPip
  • 13 posts

Posted 27 March 2014 - 10:09 PM

Now SubmitFiles() makes more sense. I'll change my code to try this. Will SubmitFiles() accept the -f parameter so that I can force a datetime and/or user as well?

#13 P4Matt

P4Matt

    Advanced Member

  • Members
  • PipPipPip
  • 1383 posts

Posted 27 March 2014 - 11:23 PM

Unfortunately no; I really wanted that when I maintained our conversion scripts years ago. You'll have to submit the files and then update the owner and date.

#14 keick

keick

    Member

  • Members
  • PipPip
  • 13 posts

Posted 28 March 2014 - 12:03 AM

Manually???

#15 P4Matt

P4Matt

    Advanced Member

  • Members
  • PipPipPip
  • 1383 posts

Posted 28 March 2014 - 03:27 PM

No, no, no. That would be terrible. The process will be:

1) Check out all your files
2) Submit them using whatever method you prefer; Change.Submit() or Client.SubmitFiles()
3) Call Repository.UpdateSubmittedChangelist() with the force option to set the user name and date on the just submitted change. Here's the sample code for doing that.

Options options = new Options(ChangeCmdFlags.Force);
Changelist c = mRepository.GetChangelist(252437, null);
c.OwnerName = "matt";
c = mRepository.UpdateSubmittedChangelist(c, options);

#16 keick

keick

    Member

  • Members
  • PipPip
  • 13 posts

Posted 28 March 2014 - 09:00 PM

Wow... Completely missed that function. Been trying to override how UpdateChangelist( :blink:) worked to accept a Force. I owe you a beer, my import is running!!! Now I have to work on adding features like rename, branching, etc...

#17 keick

keick

    Member

  • Members
  • PipPip
  • 13 posts

Posted 29 March 2014 - 08:41 PM

I still owe you that beer but... there's always a 'but'. Now that I've really had a chance to actually look at what this tool was doing, I've found another 'issue'. When I looked at my imports every other changelist was gone, and in stepping through I noticed that every other one was returning "No files to submit". Not finding anything obvious, I created a new project that simply created a new file, then made 10 revisions of that file checking each one into a new changelist in a loop. Every other one fails.

Here is my reduced code, well the important bits.

static void Main(string[] args) {
//Parse the command-line options, and if they are all there time to work
CommandLineOptions options = new CommandLineOptions();
if (CommandLine.Parser.Default.ParseArguments(args, options)) {
P4Adaptor adaptor = new P4Adaptor(options);
if (adaptor.Connect()) {
	 string filename = System.IO.Path.Combine(adaptor.ClientRootDirectory, "test.txt");
	 if (System.IO.File.Exists(filename)) {
	 System.IO.File.SetAttributes(filename, FileAttributes.Normal);
	 System.IO.File.Delete(filename);
	 }
	 for (int revisions = 0; revisions < 10; revisions++) {
	 //This will create and append to the file
	 GenerateTestFile(filename);
	 adaptor.UpdateFileAsChangelist(
	 filename,
	 revisions == 0,
	 "Comment" + revisions.ToString(),
	 DateTime.Now,
	 "bfleming");
	 }
}
}
Console.Read();
}

public void UpdateFileAsChangelist(string fileName, bool isNew, string comment, DateTime stamp, string user) {
string clientImportRoot = Path.Combine(client.Root, "importing");
if ((repository != null) && (repository.Connection.Status == ConnectionStatus.Connected)) {
//Create a new numbered changelist from the server
Changelist cl = repository.NewChangelist();
cl.Description = comment;
cl = repository.CreateChangelist(cl);
Console.WriteLine("Created changelist #{0}", cl.Id);
FileSpec fs = new FileSpec();
fs.LocalPath = new LocalPath(fileName);
if (isNew) {
	 Console.WriteLine("Adding file");
	 client.AddFiles(null, fs);
}
else {
	 Console.WriteLine("Editing file");
	 client.EditFiles(null, fs);
}
try {
	 Console.WriteLine("Submitting changelist {0}", cl.Id);
	 cl.Submit(null);
	 //Now that the changelist is submitted, override the user and datetimestamp
	 cl = repository.GetChangelist(cl.Id);
	 cl.ModifiedDate = stamp;
	 cl.OwnerName = user;
	 Options op = new Options(ChangeCmdFlags.Force);
	 Console.WriteLine("Forcing date on changelist {0}", cl.Id);
	 repository.UpdateSubmittedChangelist(cl, op);
}
catch (Exception e) {
	 Console.WriteLine(e.Message);
}
}


And here is the output

Created changelist #60
Adding file
Submitting changelist 60
No files to submit.
Created changelist #61
Editing file
Submitting changelist 61
Forcing date on changelist 61
Created changelist #62
Editing file
Submitting changelist 62
No files to submit.
Created changelist #63
Editing file
Submitting changelist 63
Forcing date on changelist 63
Created changelist #64
Editing file
Submitting changelist 64
No files to submit.
Created changelist #65
Editing file
Submitting changelist 65
Forcing date on changelist 65
Created changelist #66
Editing file
Submitting changelist 66
No files to submit.
Created changelist #67
Editing file
Submitting changelist 67
Forcing date on changelist 67
Created changelist #68
Editing file
Submitting changelist 68
No files to submit.
Created changelist #69
Editing file
Submitting changelist 69
Forcing date on changelist 69

Any ideas? I haven't tried to convert it to using the default changelist yet, because I had so many other problems initially with the default ones.

#18 P4Matt

P4Matt

    Advanced Member

  • Members
  • PipPipPip
  • 1383 posts

Posted 31 March 2014 - 09:41 PM

Here's what I think is happening:

1) The script creates a new numbered changelist. There are no files checked out so there are no files in the changelist at that time.
2) The script runs Add/Edit. No changelist is specified, so those checked out files go into the default changelist.
3) The script tries to submit the numbered changelist. The checked out files are in the default changelist, not the numbered one, so you get the error "no files to submit".
4) The script creates a new numbered changelist. There are checked out files this time, so they automatically get slurped up into the nuew numbered change.
5) The script runs add/edit. It's the same file as the one that is checked out, so nothing happens.
6) The scripts tries to submit the numbered changelist. There are files, so everything goes as planned.
Goto 1)

Try running add/edit before creating the changelist and I bet it will work.

#19 keick

keick

    Member

  • Members
  • PipPip
  • 13 posts

Posted 04 April 2014 - 11:54 AM

I think your right. As soon as I moved changelist creation to after my adds/edits it stopped skipping. With this new revelation in hand I attempted to go back to using the default changelist (after adding/editing files). That still didn't work, but with numbered changelists working I think i'm golden, so no sense in me trying to hard.

Thanks

#20 Tank

Tank

    Advanced Member

  • Members
  • PipPipPip
  • 112 posts

Posted 09 April 2014 - 11:44 AM

Bit late in the day to the party, but we tried to get on with the .NET API and found it easier to just process out to p4.exe, it has the added benefit of being able to use P4V to know what command to spawn for particular tasks. And is more fine grained because of it.




0 user(s) are reading this topic

0 members, 0 guests, 0 anonymous users