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

Iceberg method history shows only oldest commit #5578

Closed
akgrant43 opened this issue Jan 25, 2020 · 2 comments · Fixed by pharo-vcs/iceberg#1341
Closed

Iceberg method history shows only oldest commit #5578

akgrant43 opened this issue Jan 25, 2020 · 2 comments · Fixed by pharo-vcs/iceberg#1341
Assignees

Comments

@akgrant43
Copy link
Collaborator

As shown in the attached image, displaying the versions of a method shows only the earliest commit:

Screenshot from 2020-01-25 15-00-31

Iceberg: 1.6.5
Pharo-8.0.0+build.1124.sha.0932da82f08175e906b0e2a8052120c823374e9f (64 Bit)
Ubuntu 18.04

@akgrant43
Copy link
Collaborator Author

Making the following change gets it working for the example above:

IceLibgitFiletreeLogReader>>loadDefinitions
 	| entry segments className classIsMeta entryPath |	
	
	definitions := OrderedCollection new.
	
	segments := (self fileName substrings: '/') allButLast asOrderedCollection.
	classIsMeta := segments removeLast = 'class'.
	className := segments last copyUpToLast: $..
	entryPath := (segments indexOf: packageDirectory filename) > 0 ifTrue: 
		[ entryPath := $/ join: (segments copyFrom: 1 to: (segments indexOf: packageDirectory filename)).
		self fileName allButFirst: entryPath size ]
	ifFalse:
		[ self fileName allButFirst: packageDirectory filename size ].
	entry := (packageDirectory 
		entryByPath: entryPath
		ifAbsent: [ ^ nil ]).
	entry readStreamDo: [ :fileStream |
		| category source timestamp selector |

		category := fileStream nextLine.
		source := fileStream upToEnd.
 		selector := self methodSelectorFor: source.
 		timestamp := stream author name, ' ' , stream time asDateAndTime asStringYMDHM.
		definitions add: (MCMethodDefinition
			className: className
			classIsMeta: classIsMeta
			selector: selector
			category: category
			timeStamp: timestamp
			source: source) ]

I'm not yet sure if this is the correct fix (which is why there isn't a PR).

@akgrant43
Copy link
Collaborator Author

The original code is:

	segments := (self fileName substrings: '/') allButLast asOrderedCollection.
	classIsMeta := segments removeLast = 'class'.
	className := segments last copyUpToLast: $..
	
	entry := (packageDirectory 
		entryByPath: (self fileName allButFirst: packageDirectory filename size)
		ifAbsent: [ ^ nil ]).

The issue is that packageDirectory is a reference to the current package, so that the argument to #entryByPath:ifAbsent: should be relative to the package, e.g. if the file name, relative to the git repository is:

src/PackageName/ClassName.class/class/methodName.st

the argument to #entryByPath:ifAbsent: should be ClassName.class/class/methodName.st.

However packageDirectory filename is the result of calling git_tree_entry_name(), which is just the file name, not a path, .e.g. PackageName in the example above, while self fileName is the full path from the repository root, e.g. src/PackageName/ClassName.class/class/methodName.st.

The suggested change above takes this in to account and correctly removes the path up to and including the package directory.

PR on the way.

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 a pull request may close this issue.

2 participants