Apache Allura™

Forge software for hosting software projects

Brought to you by: alexluberg, brondsem, deshani, dill0wn, and 10 others

#6821 Change hg browser to get "last commit" info from hg instead of mongo

Milestone: v1.1.0

Status: closed

Owner: Cory Johns

Labels: performance (126) indexless (10) sf-2 (994)

Component: SCM

Reviewer: Dave Brondsema

Updated: 2015-08-20

Created: 2013-10-31

Creator: Cory Johns

Private: No

[#6686] changed git to pull last commit info directly from the SCM instead of mongo, and SVN already did the same. Now we just need to change the Mercurial browser to do the same.

Related

Tickets: ~~#6686~~

Discussion

Dave Brondsema - 2013-11-01

Milestone: forge-nov-15 --> forge-nov-29
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Dave Brondsema - 2013-11-15

Milestone: forge-nov-29 --> forge-dec-13
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Dave Brondsema - 2013-12-03

Size: --> 2
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Cory Johns - 2013-12-04

status: open --> in-progress

assigned_to: Cory Johns
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Cory Johns - 2013-12-06

status: in-progress --> code-review
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Cory Johns - 2013-12-06

forgehg:cj/6821
allura:cj/6821

Found an issue where errors in the thread workers could potentially block the request indefinitely. Fixed in both Hg and Git.

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Dave Brondsema - 2013-12-12

QA: Dave Brondsema
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Dave Brondsema - 2013-12-12

status: code-review --> in-progress
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Dave Brondsema - 2013-12-12

In the test, it'd be better if it was clear why the error handling would be invoked.

It'd be better to timing out rather than hanging, when something does goes wrong in the threads and isn't caught.

There's quite a bit of logic overlap between hg & git's last_commit_ids: http://pastie.org/8547562 Could that be refactored and shared?

Git's implementation specifies no_merges=True, we should be consistent with hg. hg log takes a --no-merges param; can we do this with cmdutil.walkchangerevs though? I prefer no merges, but showing merges on both would be ok too. It also seems that merge commits aren't displayed in log view (either via allura or commandline, which I don't really understand). Example: merge commit ada01d on /NEWS.txt in the SF 'coils' repo. Which makes it confusing when you see the merge commit on the tree view but not the history view.

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Dave Brondsema - 2013-12-13

Milestone: forge-dec-13 --> forge-dec-27
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Cory Johns - 2013-12-16

status: in-progress --> code-review
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Cory Johns - 2013-12-16

Rebased and force-pushed to:
allura:cj/6821
forgehg:cj/6821

Removed the merge commit filter from git because: 1) that was easier, and 2) it more closes matches what you would see from git log on the command line (and also on the history / log view on the web).

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Dave Brondsema - 2013-12-16

status: code-review --> in-progress
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Dave Brondsema - 2013-12-16

RepositoryImplementation now has two last_commit_ids methods

The new method should have a docstring explaining how a specific SCM could implement just the _get_last_commit method

forgegit.tests.model.test_repository:TestGitImplementation.test_last_commit_ids is now failing

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Cory Johns - 2013-12-17

Updated:
allura:cj/6821

Ugh. The behavior of git log with respect to merge commits appears to contradict the detailed explanation under History Simplication (specifically the bits about the default mode and --dense). Apparently, git will sometimes return a merge commit as having modified a given path, even though it doesn't list it as modified in the output from the --name-only option.

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Cory Johns - 2013-12-17

status: in-progress --> code-review
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Cory Johns - 2013-12-17

Updated:
allura:cj/6821
forgehg:cj/6821

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Dave Brondsema - 2013-12-18

status: code-review --> in-progress
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Dave Brondsema - 2013-12-18

A number of allura.tests.model.test_repo:TestLastCommit test failures, mostly due to get_changes method

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Cory Johns - 2013-12-18

status: in-progress --> code-review
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Cory Johns - 2013-12-18

allura:cj/6821

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Dave Brondsema - 2013-12-18

status: code-review --> closed
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Log in to post a comment.