Bug 511212 - Review Request: cluster-glue - reusable clustering components
Review Request: cluster-glue - reusable clustering components
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Kevin Fenzi
Fedora Extras Quality Assurance
:
Depends On:
Blocks: 509880 511246
  Show dependency treegraph
 
Reported: 2009-07-14 05:01 EDT by Andrew Beekhof
Modified: 2009-07-29 07:43 EDT (History)
6 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-07-29 07:43:30 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
kevin: fedora‑review+
tibbs: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Andrew Beekhof 2009-07-14 05:01:03 EDT
Spec URL: http://oss.clusterlabs.org/~beekhof/fedora/cluster-glue.spec

SRPM URL: http://oss.clusterlabs.org/~beekhof/fedora/cluster-glue-0.9-2.fc12.src.rpm

Description: Reusable cluster components - part of the new cluster stack

A collection of common tools that are useful for writing cluster managers such as Pacemaker.
Provides a local resource manager that understands the OCF and LSB standards, and an interface to common STONITH devices.

Background:  I've recently been hired by RedHat in order to work on Pacemaker (which requires this new package). We'd like to include it in F12 so that we can offer it as a tech preview in RHEL6. 

This package is built from the pre-release snapshots that will become 1.0.0.
Submission of the Pacemaker package will follow shortly.

Please note, this is my first package so I am looking for a sponsor.

rpmlint output:

[1] cluster-glue.x86_64: E: non-standard-dir-perm /var/lib/heartbeat/cores/nobody 0700
A standard directory should have permission set to 0755. If you get this
message, it means that you have wrong directory permissions in some dirs
included in your package.

[2] cluster-glue.x86_64: E: non-standard-dir-perm /var/lib/heartbeat/cores/root 0700
A standard directory should have permission set to 0755. If you get this
message, it means that you have wrong directory permissions in some dirs
included in your package.

[3] cluster-glue.x86_64: E: non-standard-dir-perm /var/lib/heartbeat/cores/daemon 0700
A standard directory should have permission set to 0755. If you get this
message, it means that you have wrong directory permissions in some dirs
included in your package.

[4] libcluster-glue1.x86_64: W: no-documentation
The package contains no documentation (README, doc, etc). You have to include
documentation files.

[5] libcluster-glue-devel.x86_64: W: no-dependency-on libcluster-glue/libcluster-glue-libs/liblibcluster-glue

[6] libcluster-glue-devel.x86_64: W: no-documentation
The package contains no documentation (README, doc, etc). You have to include
documentation files.

5 packages and 1 specfiles checked; 3 errors, 3 warnings.


Items 1-3 appear to be an rpmlint bug, the directories exist to record core files.
The choice of 0700 was intentional and is intended to restrict access to the core file contents to the users after which the directories are named.

I believe items 4 and 6 are erroneous as the relevant files are included in the cluster-glue package.

I believe item 5 is a bug in rpmlint as the subpackage is libcluster-glue1 and the -devel package does indeed have a dependancy on it.
Comment 1 Andrew Beekhof 2009-07-19 08:47:01 EDT
I noticed some other review requests include koji builds, so here is the one for cluster-glue:
   https://koji.fedoraproject.org/koji/taskinfo?taskID=1485160
Comment 2 Andrew Beekhof 2009-07-20 04:26:18 EDT
New spec + source rpm uploaded: 
   http://oss.clusterlabs.org/~beekhof/fedora/cluster-glue.spec
   http://oss.clusterlabs.org/~beekhof/fedora/cluster-glue-0.9-3.fc12.src.rpm

In particular, Source0 is now a reference to the package's Mercurial repo.

Conducted a self-review using http://fedoraproject.org/wiki/Packaging/ReviewGuidelines and it is my assessment that the package meets all the requirements.
Comment 3 Steven Dake 2009-07-22 15:26:58 EDT
I believe the following needs to change, but I am not an official reviewer.

1)
All RPMs created need a LICENSE file added to their respective %doc sections.  This will get rid of the annoying rpmlint warning about docs, and also follow the proper guidelines for packaging.

2)
libcluster-glue should be called cluster-glue-lib, or cluster-gluelib, or cluster-glue-libs, but not 100% positive on what the exact policy is regarding naming of library packages.  A more experienced packager may know or you could ask on the fedora developer ml or in the fedora developer channel on irc.

3)
I assume the main cluster-glue package depends on libcluster-glue, but there is no requires for that.  (this is the 5 rpmlint bug).  I don't believe rpm automatically puts dependencies in subpackages.  You have to do that yourself.

4)
libcluster-glue-devel should be called cluster-gluelib-devel or whatever you decided from step #2 above.

The rest looks good and the 0700 directory permissions make sense.
Comment 4 Andrew Beekhof 2009-07-23 03:06:36 EDT
Thanks for taking the time to look over the package. 

Regarding the review flag, is the follow from http://fedoraproject.org/wiki/Package_Review_Process incorrect?

[Quote]
Wait for someone to review your package! At this point in the process, the fedora-review flag is blank, meaning that no reviewer is assigned.
A reviewer takes on the task of reviewing your package. They will set the fedora-review flag to ?
[/Quote]

1) I saw this under the SHOULD section and decided to consider it optional.
But I think you're right, I may as well add it and make rpmlint happy. 

3) Np.  I'll specify it manually, but as binaries from cluster-glue depend on libs from libcluster-glue I had assumed that there would be an automatic dependancy created.  Is that not the case?

Also, the warning mentioned is from libcluster-glue-devel.x86_64, so I don't believe it will go away after this change.

2,4) I thought I remembered seeing that both lib%{name}  and %{name}-lib were allowed, but I can't find anything one way or the other.  If anyone knows the official policy here I'd be happy to comply.
Comment 5 Andrew Beekhof 2009-07-23 03:23:07 EDT
Quick update, while searching for the shared library naming policy, I came across this:

[Quote from https://fedoraproject.org/wiki/Packaging/Guidelines#Explicit_Requires]
We generally rely on rpmbuild to automatically add dependencies on library SONAMEs. Modern package management tools are capable of resolving such dependencies to determine the required packages.
[/Quote]

Which seems to recommend against item 3 from the review.
Comment 6 Lon Hohberger 2009-07-23 10:02:08 EDT
I probably incorrectly set it; I should clear it then.
Comment 7 Lon Hohberger 2009-07-23 10:02:53 EDT
I hear varying things sometimes - in this case, that setting it to '?' sends a message to the reviewers prompting them to do a review.
Comment 8 Andrew Beekhof 2009-07-24 05:41:46 EDT
Another update:
- Update the tarball from upstream to version 75cab275433e
- Include an AUTHORS and license file in each package
- Change the library package name to cluster-glue-libs to be more Fedora compliant

SPEC: http://oss.clusterlabs.org/~beekhof/fedora/cluster-glue.spec
SRPM: http://oss.clusterlabs.org/~beekhof/fedora/cluster-glue-0.9-4.fc12.src.rpm

Current rpmlint output follows:

[beekhof@rawhide glue]$ rpmlint /var/lib/mock/fedora-rawhide-x86_64/result/*.rpm cluster-glue.spec 
cluster-glue.x86_64: E: non-standard-dir-perm /var/lib/heartbeat/cores/nobody 0700
cluster-glue.x86_64: E: non-standard-dir-perm /var/lib/heartbeat/cores/root 0700
cluster-glue.x86_64: E: non-standard-dir-perm /var/lib/heartbeat/cores/daemon 0700
5 packages and 1 specfiles checked; 3 errors, 0 warnings.
Comment 9 Kevin Fenzi 2009-07-26 15:26:37 EDT
ok. Finally found some time to look at this a bit. ;) 

So, my understanding is that this is a newerversion/fork of heartbeat thats stripped down and only has the parts pacemaker needs/wants in it. Is that correct or am I misunderstanding? :) 

First, Fedora really would prefer to avoid Conflicts if at all possible. 
(See https://fedoraproject.org/wiki/Packaging/Conflicts). 
So, is there some way we can avoid that here? 

- Can this package just rename the dirs/files that conflict with 'cluster-glue' or something and that way they could both be installed and pacemaker could have a compile time option to use whichever one. (Or autodetect it based on the files found, etc). 

- Is using this better than using heartbeat for pacemaker? Or are they the same? If the same, perhaps we hold off on adding this to fedora for now, let pacemaker use heartbeat and then down the road when enough folks have moved to pacemaker submit this and obsolete heartbeat? 

- Perhaps pacemaker could fold in the needed parts of this and just use them internally and avoid conflict with heartbeat that way?

Thoughts?
Comment 10 Andrew Beekhof 2009-07-26 17:13:13 EDT
(In reply to comment #9)
> ok. Finally found some time to look at this a bit. ;) 

Much appreciated

> So, my understanding is that this is a newerversion/fork of heartbeat thats
> stripped down and only has the parts pacemaker needs/wants in it. Is that
> correct or am I misunderstanding? :) 

We're in the process of splitting the rest of the codebase.
Its essentially done but just needs the sort of packaging cleanup that we're doing here before it will be made available for general consumption.  I've also been on vacation a bit which slowed things down.

The idea is to split the code-base into pieces based on their usage and update frequency.  So essentially: 
  heartbeat 2.1 + features + bugfixes = cluster-glue + cluster-agents + heartbeat 3.0 + pacemaker 1.0

or more recently (and relevantly for those not using the crm):
  heartbeat 2.99 + bugfixes = cluster-glue + cluster-agents + heartbeat 3.0

This was mentioned on the mailing list late last month:
   http://www.gossamer-threads.com/lists/linuxha/dev/56290

> 
> First, Fedora really would prefer to avoid Conflicts if at all possible. 
> (See https://fedoraproject.org/wiki/Packaging/Conflicts). 
> So, is there some way we can avoid that here? 

The Conflicts was only intended to be temporary.

I didn't want to force a synchronized update to the heartbeat package just to get Pacemaker (and therefor cluster-glue) in.

The intention from upstream is that heartbeat 3.0 (which would be the messaging and membership layer as well as the v1 resource manager) would build against cluster-glue. 

If/when the cluster-glue package gets approved, I would then submit an update to the Fedora's heartbeat package and we could then remove the Conflicts directive.

On the other hand, if Fedora wanted to preserve heartbeat in its current pre-split state, I figured the Conflicts would (safely) allow that.

> 
> - Can this package just rename the dirs/files that conflict with 'cluster-glue'
> or something and that way they could both be installed and pacemaker could have
> a compile time option to use whichever one. (Or autodetect it based on the
> files found, etc). 

In theory this could be possible, but ideally it wouldn't be necessary since we'd have both Pacemaker and an updated Heartbeat both using cluster-glue as a common base.

> 
> - Is using this better than using heartbeat for pacemaker? Or are they the
> same? 

Its a subset, essentially: clplumbing + lrmd + stonith

> If the same, perhaps we hold off on adding this to fedora for now, let
> pacemaker use heartbeat and then down the road when enough folks have moved to
> pacemaker submit this and obsolete heartbeat? 

I don't think that's possible, since heartbeat 2.1 conflicts massively with Pacemaker.
I suspect there'd be all sorts of compiler problems, not least of all due to multiple copies of "crm" header files.

> - Perhaps pacemaker could fold in the needed parts of this and just use them
> internally and avoid conflict with heartbeat that way?

That was considered undesirable.

If we were intending to fork the code, then putting it in Pacemaker would make more sense.
But that would be a last possible resort.


Does that help?
Comment 11 Kevin Fenzi 2009-07-26 23:26:48 EDT
ok, that makes sense. 

So, we need to get this reviewed and in, then update heartbeat and also review/import pacemaker. 

Conflicts are nasty in that basically we have to tell the user "hey, we can't do what you want". :( I think we can possibly avoid the conflict by importing this around the same time we update heartbeat to use it. I don't know if there is a big group that wants to keep heartbeat 2.1.x in fedora, but time does march on, and fedora should be moving with it. ;) 

Anyhow, I guess I will look at doing a review here soon... ;)
Comment 12 Fabio Massimo Di Nitto 2009-07-27 07:13:39 EDT
Hi guys,

I did run through a review of cluster-glue.

rpmlint output:

[fabbione@daitarn-fedora rpmbuild]$ rpmlint SPECS/cluster-glue.spec SRPMS/cluster-glue-0.9-4.fc12.src.rpm RPMS/x86_64/cluster-glue-*
cluster-glue.x86_64: E: non-standard-dir-perm /var/lib/heartbeat/cores/nobody 0700
cluster-glue.x86_64: E: non-standard-dir-perm /var/lib/heartbeat/cores/root 0700
cluster-glue.x86_64: E: non-standard-dir-perm /var/lib/heartbeat/cores/daemon 0700
5 packages and 1 specfiles checked; 3 errors, 0 warnings.

For the record:

SHOULD: Usually, subpackages other than devel should require the base package using a fully versioned dependency. [22]

the -devel package Requires: %{name} = %{version}-%{release}

If this is done on purpose it should be documented in the spec file otherwise dropped.

SHOULD: The reviewer should test that the package builds in mock. [29]

For some reasons mock is busted in rawhide and I am unable to perform this check. The package builds fine in an installed rawhide machine.

The package passes all the checklist for ReviewGuidelines.

Kevin: I hope this can help speed up the process.

Fabio
Comment 13 Andrew Beekhof 2009-07-27 07:43:01 EDT
(In reply to comment #12)

> SHOULD: Usually, subpackages other than devel should require the base package
> using a fully versioned dependency. [22]
> 
> the -devel package Requires: %{name} = %{version}-%{release}
> 
> If this is done on purpose it should be documented in the spec file otherwise
> dropped.

I'll drop it.

> SHOULD: The reviewer should test that the package builds in mock. [29]
> 
> For some reasons mock is busted in rawhide and I am unable to perform this
> check. The package builds fine in an installed rawhide machine.

Strange, I'm running rawhide from last week and mock seems to work fine.
Comment 14 Andrew Beekhof 2009-07-27 10:45:31 EDT
New version:
- Use linux-ha.org for Source0
- Remove Requires: $name from -devel as its implied
- Instead of 'daemon', use the same user and group from the Heartbeat package and create it if necessary*

SPEC:http://oss.clusterlabs.org/~beekhof/fedora/cluster-glue.spec
SRPM: http://oss.clusterlabs.org/~beekhof/fedora/cluster-glue-0.9-5.fc12.src.rpm

* Originally I had been of the notion that we should use daemon instead of hacluster/haclient.
However for the sake of consistency, its probably better not to change the user/group. 


Looking at the Heartbeat spec, it does not create a user with a fixed id.
Is that intentional?

There is a comment in the changelog indicating that it used to, with uid=24 it seems, but that functionality seems to have been dropped somewhere along the line.

Anyway, I've preserved the Heartbeat's %pre semantics when adding it to cluster-glue.


Btw. I can no longer confirm that it builds with mock as the big rebuild seems to cause:
ERROR with rpm_check_debug vs depsolve:
rpmlib(PayloadIsXz) <= 5.2-1 is needed by glibc-devel-2.10.90-7.1.x86_64
Comment 15 Kevin Fenzi 2009-07-27 19:40:24 EDT
OK - Package meets naming and packaging guidelines
OK - Spec file matches base package name. 
OK - Spec has consistant macro usage. 
OK - Meets Packaging Guidelines. 
OK - License (GPLv2+ and LGPLv2+)
OK - License field in spec matches
See below - License file included in package
OK - Spec in American English
OK - Spec is legible.
See below - Sources match upstream md5sum:
a9aba6ae59030a148dd95bfea163852c  75cab275433e.tar.gz
ecc791dec7788293ad3d7f22b1d80cf5  75cab275433e.tar.gz.orig
OK - BuildRequires correct
OK - Package has %defattr and permissions on files is good. 
OK - Package has a correct %clean section. 
OK - Package has correct buildroot
OK - Package is code or permissible content. 
OK - Packages %doc files don't affect runtime. 
OK - Package has rm -rf RPM_BUILD_ROOT at top of %install

OK - Headers/static libs in -devel subpackage. 
OK - Spec has needed ldconfig in post and postun
OK - .so files in -devel subpackage.
OK - -devel package Requires: %{name} = %{version}-%{release}
OK - .la files are removed. 

OK - Package compiles and builds on at least one arch. 
OK - Package has no duplicate files in %files. 
OK - Package doesn't own any directories other packages own. 
OK - Package owns all the directories it creates. 
OK - Package obey's FHS standard (except for 2 exceptions)
See below - No rpmlint output. 
OK - final provides and requires are sane.

SHOULD Items:

OK - Should build in mock. 
OK - Should build on all supported archs
OK - Should have subpackages require base package with fully versioned depend. 
OK - Should have dist tag
OK - Should package latest version
OK - Should not use file requires outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin

Issues: 

1. What is the 'uid' define for? 

2. You should use %global instead of %define. 

3. Is this a pre-release for version 1.0? Or a post release of 0.9? or something else?
The release might need adjustment. See: 
https://fedoraproject.org/wiki/Packaging/NamingGuidelines#Package_Release
I suspect it should be something like: 1.0.0-0.0.75cab275433e ?

4. The %{configure} macro should pass all of these I think, so no need to do so: 
                --prefix=%{_prefix} \
                --localstatedir=%{_var} \
                --libdir=%{_libdir}

5. Your source doesn't match the upstream Source url. Perhaps you are using a checkout? 
It needs to match exactly. 

6. rpmlint says: 
cluster-glue.x86_64: E: non-standard-dir-perm /var/lib/heartbeat/cores/nobody 0700
cluster-glue.x86_64: E: non-standard-dir-perm /var/lib/heartbeat/cores/root 0700
cluster-glue.x86_64: E: non-standard-dir-perm /var/lib/heartbeat/cores/daemon 0700
5 packages and 0 specfiles checked; 3 errors, 0 warnings.

I think those can be ignored.
Comment 16 Andrew Beekhof 2009-07-28 01:35:03 EDT
(In reply to comment #15)
> a9aba6ae59030a148dd95bfea163852c  75cab275433e.tar.gz
> ecc791dec7788293ad3d7f22b1d80cf5  75cab275433e.tar.gz.orig

> Issues: 
> 
> 1. What is the 'uid' define for? 

Copied from heartbeat.spec, will remove.

> 2. You should use %global instead of %define. 

can do

> 3. Is this a pre-release for version 1.0? Or a post release of 0.9? or
> something else?

pre-release of 1.0

I'll change the version and add the hg revision as part of the alphatag.

> The release might need adjustment. See: 
> https://fedoraproject.org/wiki/Packaging/NamingGuidelines#Package_Release
> I suspect it should be something like: 1.0.0-0.0.75cab275433e ?
> 
> 4. The %{configure} macro should pass all of these I think, so no need to do
> so: 
>                 --prefix=%{_prefix} \
>                 --localstatedir=%{_var} \
>                 --libdir=%{_libdir}

Will remove and confirm.

> 5. Your source doesn't match the upstream Source url. Perhaps you are using a
> checkout? 
> It needs to match exactly. 

Mercurial embeds the repo in the tarball (in the .hg_archival.txt file).

[beekhof@rawhide Pacemaker-1-0-c9120a53a6ae]$ cat .hg_archival.txt 
repo: 8448b17e67437947c48c639c6faa4371c4a14b3b
node: c9120a53a6ae63119f96456cafb082d3c9f8a3d2

I forgot about this and only confirmed that the new url worked, not that md5sum's matched.
Consistent use of hg.linux-ha.org for all tarballs will avoid this problem.


New SRPM/SPEC to follow
Comment 17 Andrew Beekhof 2009-07-28 02:00:15 EDT
Update:
  - Use	%global	instead	of %define
  - Remove unused rpm variable
  - Remove redundant configure options
  - Change version to 1.0.0 pre-release and include Mercurial tag in version
  - md5sums now match

SPEC: http://oss.clusterlabs.org/~beekhof/fedora/cluster-glue.spec
SRPM: http://oss.clusterlabs.org/~beekhof/fedora/cluster-glue-1.0-6.75cab275433e.hg.fc12.src.rpm

I followed the svn pattern for the alphatag, however since letters appear in Mercurial tags, I added an additional decimal point.

Unfortunately, presumably due to the rebuild, I can't build anything at the moment.
For this reason, I retained --localstatedir=%{_var} as it wasn't 100% obvious that redundant.
Comment 18 Andrew Beekhof 2009-07-28 02:31:25 EDT
(In reply to comment #17)
> For this reason, I retained --localstatedir=%{_var} as it wasn't 100% obvious
> that redundant.  

I got things building (and installing) again and was able to confirm that the above does not seem to be passed by default and is therefore required.
Comment 19 Kevin Fenzi 2009-07-28 12:15:19 EDT
This looks pretty close, but the release is not quite right. You want a 0. leading there. 

So, 1.0-0.6.75cab275433e.hg

This is so that when 1.0 comes out, it can be 1.0-1.. and update cleanly from the 1.0-0.N.tag

Can you spin up an update with that and I will check the other items and confirm... ;)
Comment 20 Andrew Beekhof 2009-07-28 12:26:49 EDT
New SRPM that includes a leading zero when alphatag is defined
http://oss.clusterlabs.org/~beekhof/fedora/cluster-glue-1.0-0.7.75cab275433e.hg.fc12.src.rpm

SPEC is in the same location as normal
Comment 21 Kevin Fenzi 2009-07-28 12:41:00 EDT
ok, everything looks good with that version from what I can see. 

Sources checkout ok. 

a9aba6ae59030a148dd95bfea163852c  ./75cab275433e.tar.gz
a9aba6ae59030a148dd95bfea163852c  ./75cab275433e.tar.gz.orig

All items from above seem to be solved, so this package is APPROVED. 
I will go ahead and sponsor you now. 

Shall we wait to import this until we make changes to heartbeat? 
I will try and look at the pacemaker review soon too... if you could change the version there based on this that would be great.
Comment 22 Steven Dake 2009-07-28 12:47:10 EDT
Kevin,

Thanks for the review.  I appreciate it.
Comment 23 Andrew Beekhof 2009-07-28 12:50:11 EDT
(In reply to comment #21)

> All items from above seem to be solved, so this package is APPROVED. 
> I will go ahead and sponsor you now. 

Awesome! :-)

> Shall we wait to import this until we make changes to heartbeat? 

I think it would make reviewing pacemaker and verifying the heartbeat changes easier if it was already imported (since it would simplify mock builds and make koji --scratch possible).

So I'd vote for not waiting. 

> I will try and look at the pacemaker review soon too... if you could change the
> version there based on this that would be great.  

Already done :-)
Comment 24 Andrew Beekhof 2009-07-28 16:27:30 EDT
Requesting the creation of a CVS module as per http://fedoraproject.org/wiki/PackageMaintainers/CVSAdminProcedure
Comment 25 Andrew Beekhof 2009-07-28 16:33:06 EDT
Sorry, didnt read far enough down the page.
Missing information follows

New Package CVS Request
=======================
Package Name: cluster-glue 
Short Description: Reusable cluster components
Owners: andrew@beekhof.net
Branches: F-11 F-12
InitialCC: llh@redhat.com fdinitto@redhat.com
Comment 26 Jason Tibbitts 2009-07-28 21:04:18 EDT
Owners and InitialCC need to be FAS account names, not email addresses.
Also, we cannot yet make F-12 branches.

Please submit a corrected CVS request and re-set the fedora-cvs flag and we'll take care of things.
Comment 27 Fabio Massimo Di Nitto 2009-07-28 23:28:25 EDT
New Package CVS Request
=======================
Package Name: cluster-glue 
Short Description: Reusable cluster components
Owners: andrew@beekhof.net
Branches: F-11
InitialCC: lon fabbione
Comment 28 Fabio Massimo Di Nitto 2009-07-28 23:39:29 EDT
not enough coffee:

New Package CVS Request
=======================
Package Name: cluster-glue 
Short Description: Reusable cluster components
Owners: beekhof
Branches:
InitialCC: lon fabbione

(I also confirm that we don't need F-11 branch)
Comment 29 Jason Tibbitts 2009-07-28 23:50:33 EDT
CVS done.
Comment 30 Andrew Beekhof 2009-07-29 07:43:30 EDT
Built and tagged:
   http://koji.fedoraproject.org/koji/buildinfo?buildID=124411 

Closing.  Thanks everyone!

Note You need to log in before you can comment on or make changes to this bug.