Skip to content

Allow different repodir in scripts#69

Open
Whissi wants to merge 3 commits into
gentoo:masterfrom
Whissi:allow-different-portdir-in-scripts
Open

Allow different repodir in scripts#69
Whissi wants to merge 3 commits into
gentoo:masterfrom
Whissi:allow-different-portdir-in-scripts

Conversation

@Whissi

@Whissi Whissi commented Dec 3, 2020

Copy link
Copy Markdown
Contributor

I call tatt on the x86 system where I do AT work but I do not commit from this system. Therefore I set different repodir in tatt config. But since commit 042010b this doesn't work anymore because repodir value is now also used for nattka and must point to a valid repository.

This PR will partially revert commit 042010b because it doesn't make sense to call a helper function multiple times. Instead tatt will now define repodir if not set and validate value at start after config loading.

In addition an additional option repodir_commits was introduced which will get used in commit-header template and defaults to repodir value.

Fixes 042010b ("tool: Refactor repodir handling")
Signed-off-by: Thomas Deutschmann <whissi@gentoo.org>
This commit is basically reverting commit 042010b.

Instead calling a helper function all the time we need repodir value,
define/validate value once at start.

Signed-off-by: Thomas Deutschmann <whissi@gentoo.org>
This will allow for running commit scripts on a different host.

Signed-off-by: Thomas Deutschmann <whissi@gentoo.org>
@Whissi Whissi requested a review from thesamesam December 3, 2020 01:54
@thesamesam

Copy link
Copy Markdown
Member

Thanks, I was going to message you and double-check what the issue remaining was.

Regarding dropping the repodir function, I did it because the tatt script is a monolith right now, but I also accept that I can't think of another use for it right now! So we can always return to it if we need to.

@DerDakon

DerDakon commented Dec 3, 2020

Copy link
Copy Markdown
Contributor

I run the tests in chroot, and do the commits outside or even on a different machine using a crude script inherited from @trofi and then heavily modified. I would welcome if someone comes up with a better solution, like writing the nattka commits to a script file so I can move them over or whatever.

Comment thread scripts/tatt
# Now that we use nattka, we need a working repodir
if config['repodir'] == "":
if os.path.isdir("/var/db/repos/gentoo"):
config['repodir']="/var/db/repos/gentoo"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

spaces around =

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I adopted existing coding style, see assignment in line 92 and 94 above.

@DerDakon

DerDakon commented Dec 3, 2020

Copy link
Copy Markdown
Contributor

You can easily get the same result if you overwrite your commit-header. I usually have a ~/tatt in my build chroots and symlink all files from /usr/share into that and overwrite those that need it.

@Whissi

Whissi commented Dec 3, 2020

Copy link
Copy Markdown
Contributor Author

Yeah, providing your own templates is another 'solution' for my problem. I don't like it or try to avoid in case templates will be updated: In this case I will probably not notice the changes. So I prefer using default templates but customize via option.

@thesamesam thesamesam left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Note that get_repo_dir was a function to isolate the logic (to make it easier to find and modify) and cleanup the monolithic script file, so I don't think it was worth reverting it.

Looks fine otherwise, I'll merge it if you can fix the merge conflict. Ideally, undo the revert for aforementioned reasons.

I agree this is a good use for templates, but given I hit the same use case, I guess it's common enough to justify a config option.

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.

3 participants