Skip to content

Conversation

@Whissi
Copy link
Contributor

@Whissi Whissi commented Dec 3, 2020

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
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
Copy link
Contributor

DerDakon commented Dec 3, 2020

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.

# 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
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
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
Copy link
Contributor

DerDakon commented Dec 3, 2020

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
Copy link
Contributor Author

Whissi commented Dec 3, 2020

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.

Copy link
Member

@thesamesam thesamesam left a comment

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