Skip to contentSkip to conversation

scripts/get_maintainer.pl misses maintainers sometimes

PATCH Maarten Lankhorst 2 participants 3 messages Jul 12, 2017 – Jul 13, 2017

Subsystems (14)
DRM drivers DRM misc Intel i915 Radeon and amdgpu DRM drivers +10 more subsystems
lore
Share & tools
lore

Maarten Lankhorst

author maintainer
#0 lore raw

Hello Joe Perches,

I created a script for drm's maintainer-tools that pipes the output of get_maintainer.pl to add the appropriate cc's to the commit message. It also ignores duplicates, so running the script twice on the same commit doesn't add everyone twice.

When testing, I found out that sometimes maintainers were added on the second patch, and that made me notice a bug in get_maintainer.pl.

On the below patch, I get up to 39 maintainers from get_maintainer.pl, but the actual amount differs randomly on each invocation:

~/linux$ git show | scripts/get_maintainer.pl | wc -l 39
~/linux$ git show | scripts/get_maintainer.pl | wc -l 37
~/linux$ git show | scripts/get_maintainer.pl | wc -l 38

Any idea why this happens?

The commit I used for testing is pasted below.

Kind regards,
Maarten Lankhorst

--8<--
commit 94273dc6f6e3ab77948dddc6e0572ca7ea890520

Author: Daniel Vetter <[email protected]>
Date:   Wed Jun 28 14:11:49 2017 +0200

drm/: Drop fbdev info flags

- FBINFO_CAN_FORCE_OUTPUT has been a lie ever since we nerfed&removed the entire panic handling code in our fbdev emulation. We might restore kms panic output, but not through the bazillion of legacy code layers called fbdev/fbcon, there's just no way to make that work safely.

- With the module check change FBINFO_DEFAULT is always 0, so can be removed too.

That removes another change to cargo-cult stuff in kms drivers, yay!

Signed-off-by Daniel Vetter <[email protected]>
drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c +0 -1
index c0d8c6ff6380..1c57fefc364c 100644
@@ -245,7 +245,6 @@ static int amdgpufb_create(struct drm_fb_helper *helper,
 
 	drm_fb_helper_fill_fix(info, fb->pitches[0], fb->format->depth);
 
-	info->flags = FBINFO_DEFAULT | FBINFO_CAN_FORCE_OUTPUT;
 	info->fbops = &amdgpufb_ops;
 
 	tmp = amdgpu_bo_gpu_offset(abo) - adev->mc.vram_start;
drivers/gpu/drm/armada/armada_fbdev.c +0 -1
index 602dfead8eef..5b479b0ed06c 100644
@@ -81,7 +81,6 @@ static int armada_fb_create(struct drm_fb_helper *fbh,
 
 	strlcpy(info->fix.id, "armada-drmfb", sizeof(info->fix.id));
 	info->par = fbh;
-	info->flags = FBINFO_DEFAULT | FBINFO_CAN_FORCE_OUTPUT;
 	info->fbops = &armada_fb_ops;
 	info->fix.smem_start = obj->phys_addr;
 	info->fix.smem_len = obj->obj.size;
drivers/gpu/drm/ast/ast_fb.c +0 -1
index 4ad4acd0ccab..53ca6d099234 100644
@@ -231,7 +231,6 @@ static int astfb_create(struct drm_fb_helper *helper,
 
 	strcpy(info->fix.id, "astdrmfb");
 
-	info->flags = FBINFO_DEFAULT | FBINFO_CAN_FORCE_OUTPUT;
 	info->fbops = &astfb_ops;
 
 	info->apertures->ranges[0].base = pci_resource_start(dev->pdev, 0);
drivers/gpu/drm/bochs/bochs_fbdev.c +0 -1
index 49d5a2b7d630..14eb8d0d5a00 100644
@@ -118,7 +118,6 @@ static int bochsfb_create(struct drm_fb_helper *helper,
 
 	strcpy(info->fix.id, "bochsdrmfb");
 
-	info->flags = FBINFO_DEFAULT;
 	info->fbops = &bochsfb_ops;
 
 	drm_fb_helper_fill_fix(info, fb->pitches[0], fb->format->depth);
drivers/gpu/drm/cirrus/cirrus_fbdev.c +0 -1
index 7fa58eeadc9d..c69586163d92 100644
@@ -215,7 +215,6 @@ static int cirrusfb_create(struct drm_fb_helper *helper,
 
 	strcpy(info->fix.id, "cirrusdrmfb");
 
-	info->flags = FBINFO_DEFAULT;
 	info->fbops = &cirrusfb_ops;
 
 	drm_fb_helper_fill_fix(info, fb->pitches[0], fb->format->depth);
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c +0 -1
index f5ac80daeef2..9740eed9231a 100644
@@ -131,7 +131,6 @@ static int hibmc_drm_fb_create(struct drm_fb_helper *helper,
 
 	strcpy(info->fix.id, "hibmcdrmfb");
 
-	info->flags = FBINFO_DEFAULT;
 	info->fbops = &hibmc_drm_fb_ops;
 
 	drm_fb_helper_fill_fix(info, hi_fbdev->fb->fb.pitches[0],
drivers/gpu/drm/i915/intel_fbdev.c +0 -1
index f11ee039ff45..b5fee2b59d67 100644
@@ -232,7 +232,6 @@ static int intelfb_create(struct drm_fb_helper *helper,
 
 	strcpy(info->fix.id, "inteldrmfb");
 
-	info->flags = FBINFO_DEFAULT | FBINFO_CAN_FORCE_OUTPUT;
 	info->fbops = &intelfb_ops;
 
 	/* setup aperture base/size for vesafb takeover */
drivers/gpu/drm/mgag200/mgag200_fb.c +0 -1
index 5d3b1fac906f..e94d78a32fe0 100644
@@ -210,7 +210,6 @@ static int mgag200fb_create(struct drm_fb_helper *helper,
 
 	strcpy(info->fix.id, "mgadrmfb");
 
-	info->flags = FBINFO_DEFAULT | FBINFO_CAN_FORCE_OUTPUT;
 	info->fbops = &mgag200fb_ops;
 
 	/* setup aperture base/size for vesafb takeover */
drivers/gpu/drm/msm/msm_fbdev.c +0 -1
index 5ecf4ff9a059..9c00fedfc741 100644
@@ -143,7 +143,6 @@ static int msm_fbdev_create(struct drm_fb_helper *helper,
 	helper->fb = fb;
 
 	fbi->par = helper;
-	fbi->flags = FBINFO_DEFAULT;
 	fbi->fbops = &msm_fb_ops;
 
 	strcpy(fbi->fix.id, "msm");
drivers/gpu/drm/omapdrm/omap_fbdev.c +0 -1
index daf81a0a2899..9273118040b7 100644
@@ -184,7 +184,6 @@ static int omap_fbdev_create(struct drm_fb_helper *helper,
 	helper->fb = fb;
 
 	fbi->par = helper;
-	fbi->flags = FBINFO_DEFAULT;
 	fbi->fbops = &omap_fb_ops;
 
 	strcpy(fbi->fix.id, MODULE_NAME);
drivers/gpu/drm/qxl/qxl_fb.c +0 -1
index 573e7e9a5f98..844c4a31ca13 100644
@@ -275,7 +275,6 @@ static int qxlfb_create(struct qxl_fbdev *qfbdev,
 
 	drm_fb_helper_fill_fix(info, fb->pitches[0], fb->format->depth);
 
-	info->flags = FBINFO_DEFAULT | FBINFO_HWACCEL_COPYAREA | FBINFO_HWACCEL_FILLRECT;
 	info->fbops = &qxlfb_ops;
 
 	/*
drivers/gpu/drm/radeon/radeon_fb.c +0 -1
index 356ad90a5238..e141fcd5e8e1 100644
@@ -264,7 +264,6 @@ static int radeonfb_create(struct drm_fb_helper *helper,
 
 	drm_fb_helper_fill_fix(info, fb->pitches[0], fb->format->depth);
 
-	info->flags = FBINFO_DEFAULT | FBINFO_CAN_FORCE_OUTPUT;
 	info->fbops = &radeonfb_ops;
 
 	tmp = radeon_bo_gpu_offset(rbo) - rdev->mc.vram_start;
drivers/gpu/drm/udl/udl_fb.c +0 -1
index 4a6500362564..a5c54dc60def 100644
@@ -393,7 +393,6 @@ static int udlfb_create(struct drm_fb_helper *helper,
 	info->fix.smem_len = size;
 	info->fix.smem_start = (unsigned long)ufbdev->ufb.obj->vmapping;
 
-	info->flags = FBINFO_DEFAULT | FBINFO_CAN_FORCE_OUTPUT;
 	info->fbops = &udlfb_ops;
 	drm_fb_helper_fill_fix(info, fb->pitches[0], fb->format->depth);
 	drm_fb_helper_fill_var(info, &ufbdev->helper, sizes->fb_width, sizes->fb_height);
drivers/gpu/drm/virtio/virtgpu_fb.c +0 -1
index 33df067b11c1..046e28b69d99 100644
@@ -273,7 +273,6 @@ static int virtio_gpufb_create(struct drm_fb_helper *helper,
 	vfbdev->helper.fb = fb;
 
 	strcpy(info->fix.id, "virtiodrmfb");
-	info->flags = FBINFO_DEFAULT;
 	info->fbops = &virtio_gpufb_ops;
 	info->pixmap.flags = FB_PIXMAP_SYSTEM;
drivers/gpu/drm/vmwgfx/vmwgfx_fb.c +0 -1
index 6f4cb4678cbc..d23a18aae476 100644
@@ -779,7 +779,6 @@ int vmw_fb_init(struct vmw_private *vmw_priv)
 	info->screen_base = (char __iomem *)par->vmalloc;
 	info->screen_size = fb_size;
 
-	info->flags = FBINFO_DEFAULT;
 	info->fbops = &vmw_fb_ops;
 
 	/* 24 depth per default */
#1 lore raw
Quoted 12 lines · Maarten Lankhorst

Hello Joe Perches,

I created a script for drm's maintainer-tools that pipes the output of get_maintainer.pl to add the appropriate cc's to the commit message. It also ignores duplicates, so running the script twice on the same commit doesn't add everyone twice.

When testing, I found out that sometimes maintainers were added on the second patch, and that made me notice a bug in get_maintainer.pl.

On the below patch, I get up to 39 maintainers from get_maintainer.pl, but the actual amount differs randomly on each invocation:

Quoted 8 lines

~/linux$ git show | scripts/get_maintainer.pl | wc -l 39
~/linux$ git show | scripts/get_maintainer.pl | wc -l 37
~/linux$ git show | scripts/get_maintainer.pl | wc -l 38

Any idea why this happens?

If you add --nogit --nogit-fallback to the command line the output is consistent.

So it seems it comes down to that what git log outputs for the same command varies.

I'll track it down further later.

#2 lore raw
Quoted 22 lines · Joe Perches
Quoted 12 lines · Maarten Lankhorst

Hello Joe Perches,

I created a script for drm's maintainer-tools that pipes the output of get_maintainer.pl to add the appropriate cc's to the commit message. It also ignores duplicates, so running the script twice on the same commit doesn't add everyone twice.

When testing, I found out that sometimes maintainers were added on the second patch, and that made me notice a bug in get_maintainer.pl.

On the below patch, I get up to 39 maintainers from get_maintainer.pl, but the actual amount differs randomly on each invocation:

Quoted 8 lines

~/linux$ git show | scripts/get_maintainer.pl | wc -l 39
~/linux$ git show | scripts/get_maintainer.pl | wc -l 37
~/linux$ git show | scripts/get_maintainer.pl | wc -l 38

Any idea why this happens?

Quoted 7 lines

If you add --nogit --nogit-fallback to the command line the output is consistent.

So it seems it comes down to that what git log outputs for the same command varies.

I'll track it down further later.

It doesn't actually seem like a bug so much as
unreproducibility of the script output.

It seems to come down to this line in the script in subroutine vcs_assign:

    foreach my $line (sort {$hash{$b} <=> $hash{$a}} keys %hash) {

where there is a hash of sorted commit signers and this is selecting the ones with the most signatures.

When there are multiple signers with the same number of commits, perl is picking a random entry.

I think it's not particularly important to be reproducible as the MAINTAINERS entries should be prioritized and the git commit signers should only be considered when there is no listed maintainer.

If you have a bright idea as to how perl should order the entries in the hash for reproducibility, let me know.

cheers, Joe