Ticket #1216 (closed bug: fixed)

Opened 7 years ago

Last modified 7 years ago

[PATCH] Fix dependency in docs/Makefile

Reported by: doughera Owned by: jkeenan
Priority: normal Milestone:
Component: build Version: 1.7.0
Severity: medium Keywords:
Cc: Language:
Patch status: new Platform:

Description

The attached patch fixes a dependency in docs/Makefile. Obviously, you can't build the docs/ops/*.pod files until the docs/ops directory is built.

Attachments

tt1216-docs-makefile.patch Download (0.6 KB) - added by doughera 7 years ago.
tt1216-try2.patch Download (1.2 KB) - added by doughera 7 years ago.

Change History

Changed 7 years ago by doughera

follow-up: ↓ 2   Changed 7 years ago by jkeenan

  • status changed from new to assigned
  • owner set to jkeenan

Andy, I'm probably missing something here, but I don't understand what the problem is that the patch is fixing.

I applied the patch on my Linux VM, configured, built and installed Parrot underneath my homedir. I was able to locate this directory in the installed version: 

$ ls  share/doc/parrot/1.7.0-devel/pod/ops/
bit.pod  core.pod   experimental.pod  math.pod    pmc.pod  string.pod  var.pod
cmp.pod  debug.pod  io.pod            object.pod  set.pod  sys.pod

I then configured on my laptop without applying the patch, built and installed Parrot locally. Issuing the same command, I got the same results: 

$ ls share/doc/parrot/1.7.0-devel/pod/ops/
bit.pod          experimental.pod pmc.pod          var.pod
cmp.pod          io.pod           set.pod
core.pod         math.pod         string.pod
debug.pod        object.pod       sys.pod

What am I missing? What should be different or improved once the patch is applied?

Thank you very much.

kid51

in reply to: ↑ 1 ; follow-up: ↓ 5   Changed 7 years ago by doughera

  • patch set to new

Replying to jkeenan:

What am I missing? What should be different or improved once the patch is applied?

Hmm. email2trac appears to have dropped my reply. It correctly shows up on the parrot-tickets list at  http://lists.parrot.org/pipermail/parrot-tickets/Week-of-Mon-20091102/005432.html but it didn't show up here in Trac. Let me try again.

It's a dependency problem. The old version had: 

    all: doc-prep packfile-c.pod $(POD)

The doc-prep target creates the ops/ directory. The $(POD) target puts stuff in the ops/ directory. Obviously, it can't do that if the directory doesn't exist yet. A make program is free to attempt all three targets in any order. A parallel make program is free to try to make them all at the same time. If the $(POD) target starts running before the doc-prep target, it will fail.

My patch adds the dependency 

    $(POD): doc-prep

indicating that you can't make the $(POD) files in the ops/ directory until you've created the directory.

I can think of three reasons why others might not have seen this.

1. the docs/ops/ directory exists in a fresh svn checkout, but not in a release. Since the directory already exists in the svn checkout, there is no problem in the svn checkout.

2. The docs/ops directory is created by the build process (if needed) but is not removed by 'make distclean'. Thus cleaning and rebuilding doesn't expose the problem.

3. I suspect GNU might evaluate such targets left to right, so they get executed in the order you expect. However, such order is not guaranteed. Even with GNU make, if there is a parallel make, there is no guarantee that the doc-prep would be done before the $(POD) target started.

In short, my patch is designed to ensure that the directory exists before any documents are placed in it.

  Changed 7 years ago by coke 

>  Hmm.  email2trac appears to have dropped my reply.  It correctly shows up
>  on the parrot-tickets list at http://lists.parrot.org/pipermail/parrot-
>  tickets/Week-of-Mon-20091102/005432.html but it didn't show up here in
>  Trac.
>  Let me try again.

parrot-tickets is the human side; tickets is the automated side; you
probably want to hit both. This should happen automatically. when you
hit reply-all.

--
Will "Coke" Coleda

  Changed 7 years ago by doughera 

On Wed, 25 Nov 2009, Will Coleda wrote:

> >  Hmm.  email2trac appears to have dropped my reply.  It correctly shows up
> >  on the parrot-tickets list at http://lists.parrot.org/pipermail/parrot-
> >  tickets/Week-of-Mon-20091102/005432.html but it didn't show up here in
> >  Trac.
> >  Let me try again.
> 
> parrot-tickets is the human side; tickets is the automated side; you
> probably want to hit both. This should happen automatically. when you
> hit reply-all.

Ah.  Thanks for the explanation.  I would sum up my confusion this way:

    You are in a little maze of twisty ticket systems, all different.

-- 
    Andy Dougherty		doughera@lafayette.edu

in reply to: ↑ 2 ; follow-up: ↓ 6   Changed 7 years ago by jkeenan

Replying to doughera:

Replying to jkeenan:

Patch applied in r42898. I'll keep ticket open for 7 days for any further comments or complaints. Thank you very much.

kid51

in reply to: ↑ 5 ; follow-up: ↓ 7   Changed 7 years ago by japhb

Replying to jkeenan:

Replying to doughera:

Replying to jkeenan:

Patch applied in r42898. I'll keep ticket open for 7 days for any further comments or complaints. Thank you very much.

r42898 breaks 'make; sudo make install', because the .pod files now get redundantly rebuilt during the 'sudo make install' -- and since the first make sets the file modes to 0644, and perldoc drops privileges to 'nobody', during the 'sudo make install' ironically perldoc lacks privileges to do the redundant rebuild.

Please fix or revert this change; I'm not enough of a portable makefile wizard to guess the correct incantation to make it Just Work on all our platforms myself.

-'f

in reply to: ↑ 6   Changed 7 years ago by jkeenan

Replying to japhb:

Please fix or revert this change; I'm not enough of a portable makefile wizard to guess the correct incantation to make it Just Work on all our platforms myself.

Reverted in r42900.

  Changed 7 years ago by doughera

I'm sorry it caused a problem. Other stuff gets rebuilt too, so I hadn't worried about it, but I hadn't thought about the sudo/perldoc combination.

Anyway, the original problem is still there. You can't create the files in the ops/ directory until the ops/ directory exists.

The simplest solution would seem to be to include the ops/ directory in MANIFEST and hence in the release tarball. Alas that runs afoul of ExtUtils::Manifest.pm's manicheck() routine, which assumes that only files are listed in the MANIFEST.

I've attached a patch which implements a second simple solution, namely actually creating a dummy file named 'doc-prep'. This means everything now is a proper dependency and gets built when it is needed, but doesn't get rebuilt when it isn't needed.

Changed 7 years ago by doughera

  Changed 7 years ago by japhb

This second patch appears to have fixed my concern; I have committed it, thanks doughera++ !

-'f

follow-up: ↓ 11   Changed 7 years ago by Util

Added doc-prep to svn:ignore for docs/ in r42948.

in reply to: ↑ 10 ; follow-up: ↓ 12   Changed 7 years ago by jkeenan

Replying to Util:

Added doc-prep to svn:ignore for docs/ in r42948.

I'll keep this ticket open for 2-3 days more for any other comments or complaints.

kid51

in reply to: ↑ 11   Changed 7 years ago by jkeenan

  • status changed from assigned to closed
  • resolution set to fixed

Replying to jkeenan:

Replying to Util:

Added doc-prep to svn:ignore for docs/ in r42948.

I'll keep this ticket open for 2-3 days more for any other comments or complaints.

No comments or complaints. Thanks to doughera, japhb and Util for contributing to the resolution of this problem.

kid51

Note: See TracTickets for help on using tickets.