Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge pulplift #440

Merged
merged 7 commits into from Oct 26, 2020
Merged

Merge pulplift #440

merged 7 commits into from Oct 26, 2020

Conversation

mdellweg
Copy link
Member

@mdellweg mdellweg commented Sep 18, 2020

At this point in time, this is an experiment.

@pulpbot
Copy link
Member

pulpbot commented Sep 18, 2020

WARNING!!! This PR is not attached to an issue. In most cases this is not advisable. Please see our PR docs for more information about how to attach this PR to an issue.

@mikedep333
Copy link
Member

@mdellweg In the past, I have merged history from 1 repo into another. It's easy to do for a distinct set of files, even if it means putting all of the contents of the pulplift repo into a subfolder in pulp_installer (permanently or temporarily.)

I want us to try do that to merge pulplift into pulp_installer.

I think these are the instructions I followed:
https://medium.com/@ayushya/move-directory-from-one-repository-to-another-preserving-git-history-d210fa049d4b

@mdellweg
Copy link
Member Author

@mdellweg In the past, I have merged history from 1 repo into another. It's easy to do for a distinct set of files, even if it means putting all of the contents of the pulplift repo into a subfolder in pulp_installer (permanently or temporarily.)

I want us to try do that to merge pulplift into pulp_installer.

I think these are the instructions I followed:
https://medium.com/@ayushya/move-directory-from-one-repository-to-another-preserving-git-history-d210fa049d4b

That sounds like a lot of work, and i'd need to know now which files i want to move over...
At least i want to see this one green before attempting such a thing.

@mdellweg mdellweg force-pushed the merge_pulplift branch 10 times, most recently from ba8e581 to 436d532 Compare September 29, 2020 10:46
@mdellweg mdellweg marked this pull request as ready for review September 29, 2020 11:03
@mdellweg mdellweg force-pushed the merge_pulplift branch 3 times, most recently from c3980c6 to 1cf934c Compare October 15, 2020 08:17
@mdellweg
Copy link
Member Author

@mdellweg In the past, I have merged history from 1 repo into another. It's easy to do for a distinct set of files, even if it means putting all of the contents of the pulplift repo into a subfolder in pulp_installer (permanently or temporarily.)
I want us to try do that to merge pulplift into pulp_installer.
I think these are the instructions I followed:
https://medium.com/@ayushya/move-directory-from-one-repository-to-another-preserving-git-history-d210fa049d4b

That sounds like a lot of work, and i'd need to know now which files i want to move over...
At least i want to see this one green before attempting such a thing.

@mikedep333
What you suggest is to rewrite all the history of the files we are taking from the other repository, which (i think) will tear most of those commits out of their context. I do not think this is especially useful.
I could see two ways going forward here:

  1. Just take the files we want, make a clean "We add this" commit and archive the other repo for history.
  2. Pull all history (unmodified) into this repository (did that before; it's possible) and move the files around accordingly. This would conserve all the information why what is where in the git history.

As i heard people dislike complicated commit trees several times in the past we should probably pick 1.

Copy link
Member

@mikedep333 mikedep333 left a comment

Choose a reason for hiding this comment

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

Comment on lines +5 to +6
[ssh_connection]
pipelining = True
Copy link
Member

Choose a reason for hiding this comment

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

This is a setting that speeds things up greatly, but breaks a small % of users running pulp_installer directly from the pulp_installer folder.

But now that the official install instructions are to run from an installed collection, we can do this safely :)

So I like this change, and an FYI & thank you that your Collection work enabled this.

@@ -4,19 +4,26 @@
- name: Load the SELinux policy packages
command: 'semodule -i /usr/local/share/selinux/{{ ansible_facts.selinux.type }}/{{ item }}.pp'
loop: '{{ __pulp_selinux_policy_pkgs }}'
become: true
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, how the heck did these work previously? I think this may fix a recently reported issue by a user.

These are worthy of a changelog entry.

Copy link
Member Author

Choose a reason for hiding this comment

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

The ansible.cfg in pulplift contains a "become=yes" that seemed wrong to me.

Copy link
Member Author

Choose a reason for hiding this comment

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

Do we have a ticket?

Copy link
Member Author

Choose a reason for hiding this comment

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

@@ -0,0 +1,25 @@
---
Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately, there is now 1 new commit on the pulplift master branch that we need.

https://github.com/pulp/pulplift/pull/108/files

(One of them adds a new file in this directory,)

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh yes, that's unfortunate...
I will have a look tomorrow.

@mdellweg
Copy link
Member Author

@mikedep333 CI failure is unrelated.
pulp/pulp_rpm#1876

@mdellweg mdellweg merged commit ba4b60b into pulp:master Oct 26, 2020
@mdellweg mdellweg deleted the merge_pulplift branch October 26, 2020 08:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants