Skip to content
This repository has been archived by the owner on Oct 15, 2022. It is now read-only.

Check DDG before installing bundles #266

Merged
merged 7 commits into from
Aug 19, 2015
Merged

Check DDG before installing bundles #266

merged 7 commits into from
Aug 19, 2015

Conversation

zachthompson
Copy link
Contributor

Adds DDG to list of packages to be checked when bundles are in play. Currently doesn't do this and fails when installing a bundle directly or checking requirements.

Fixed a few bugs in DuckPAN::Perl, tried to clarify the what the versions really mean, and added a bit more error checking.

DuckPAN::Perl:
	* Do not ignore pin
	* Do not go into reinstall mode unless the user specified it
	* Check system call to cpanm
	* Clarify variable names and functions
my $localver = $self->get_local_version($package);
my $duckpan_module_version = version->parse($module->version);
my $duckpan_module_url = $self->app->duckpan . 'authors/id/' . $module->distribution->pathname;
my $pinned_version = $ENV{$sp};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: the indentation here is a bit too much


$localver ||= 1e-6 if ($pin_version); # a silly, but true, value if missing and we need to compare with pinned.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thin we still need this? Or we need a better way to handle the locally installed version. When I run duckpan reinstall now, I'm getting "[FATAL] Failed to find version 9.999 of App::DuckPAN".

We set the version to 9.999 if it's a local build.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line actually doesn't prevent that. When I reconsidered what reinstall should do I wanted to limit it to literally reinstalling the installed version. It doesn't quite get there because the same if block handles the "not installed" case as well but it at least defaults to it (line 125.) So you're trying to reinstall a version that's essentially not installed. That is a fatal error imo. How could it possibly reinstall that?

If you replace DuckPAN.pm and Perl.pm (or install it), you should see the desired behavior. We could also check for 9.999 in this block and skip it. Except for updating the reinstall logic and needing to test, are we really going to be doing a lot of reinstalling with local builds?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the name of the function should really be "reset" not so much "reinstall". It's meant to get a DDG dev back to where prod is.

Right this moment, I'm in need of this functionality: I've been doing both Spice and Goodie builds (and testing this PR) and so my installed Spice, Goodie and DuckPAN packages are out of sync with prod. Also the version numbers, because I've installed locally are 1 higher than the current release.

Now that I want to test a prod release of the IA's I need to "reinstall" the currently released repos and so I would typically run "duckpan reinstall" to get all my repos back to where prod is at.

Does that clarify things?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really. So why can't you duckpan upgrade to do that? Updating to the latest doesn't get you to prod because prod will often be pinned lower than latest. The next time you upgrade you are mysteriously downgraded to pinned versions. That is not good behavior. Are you saying that you want to get the latest without updating your pins to match?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe duckpan upgrade will work in that case because my localver will be higher than anything released?

The next time you upgrade you are mysteriously downgraded to pinned versions. That is not good behavior.

Agreed that behaviour sucks and has bitten me. I keep getting downgraded whenever I run duckpan server now. We should kill that.

Are you saying that you want to get the latest without updating your pins to match?

I thought so, but it might not be necessary. I suppose I can just update my pins.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe duckpan upgrade will work in that case because my localver will be higher than anything released?

You're talking about 9.999, right? The only time that happens is when you're using a repo for lib somehow, e.g. developing duckpan. We could think about something like duckpan latest but externally that's what upgrade does; internally it would just allow you to circumvent the pins which I don't think we want to do.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're talking about 9.999, right?

Nope. If I dzil install the repo because I'm developing in it then my installed version of the repo would be 1 higher than latest. So IIRC duckpan upgrade won't have any effect as the installed version is higher than the currently released/pinned version.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, again, that's a special case. In that case it should say you have a newer version and leave it alone. What behavior are you expecting? All of these are internal development edge cases imo. If you're dzil installing you're going under the hood and should be able get out of it, e.g. use cpanm -U first on you non-existent version and then duckpan upgrade.

Like I said, we could do duckpan latest. In this case it would be like a reinstall but it would try to install pinned or latest versions, ignoring whatever mess was created during development, e.g. installed_version.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of these are internal development edge cases imo.

I totally agree, it's really only a "problem" I face, but it happens very often, so I need an easy way to "fix" it.

Like I said, we could do duckpan latest. In this case it would be like a reinstall but it would try to install pinned or latest versions, ignoring whatever mess was created during development, e.g. installed_version.

That works for me 👍

@zachthompson
Copy link
Contributor Author

latest command added which basically ignores installed versions.

$_ =~ /^app/i) {
if (/^www/i ||
/^dist/i ||
/^(ddg)$/i ||
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops we need to also accept ^ddg:: so that we handle duckpan DDG::Publisher, right now that commands falls through to the emit_and_exit below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

@moollaza
Copy link
Member

LGTM 👍

moollaza added a commit that referenced this pull request Aug 19, 2015
Check DDG before installing bundles
@moollaza moollaza merged commit f1080fd into master Aug 19, 2015
@moollaza moollaza deleted the zach/req-order branch August 19, 2015 18:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants