Skip to content

Conversation

peterdell
Copy link
Contributor

Hello tebe,
as discussed here the first part of the compatibility changes for the PAS2JS target.

  • Use regular records instead of variant records
  • Use TString instead of ^TString for names in TToken
    Kind regards, Peter.

@zbyti
Copy link
Collaborator

zbyti commented Feb 24, 2025

I see that the

- name: Push changes
  run: git push origin HEAD:main

for Linux binary release needs to be fixed, because pull requests report an error in the testing procedure.

Unfortunately I don't have time for this right now...

@peterdell
Copy link
Contributor Author

I am not familiar with the build system and I cannot see what the error actually is.

@zbyti
Copy link
Collaborator

zbyti commented Feb 24, 2025

I am not familiar with the build system and I cannot see what the error actually is.

Everything is OK with your PR, I didn't take this case into account, for now there is no reason to worry about this error.

@t-w
Copy link
Collaborator

t-w commented Feb 24, 2025

That job (action "Push to repository on success") could fail on various reasons. one of them can be that somebody updated the repo in the meantime while the build job was running (what is not anything unusual...).

Btw. regarding CI - I'd be a bit more careful with automatic commits. Master branch is somewhat protected (at least from force push), but imagine what a broken build could do if branch protection is ever removed and someone changed git push to git push --force in the workflow...

Also, I would be careful with automatic committing compiled binaries to the repo (I see that started recently with e730f11) - there is a danger that the repo may start growing very fast since binaries are stored in GIT as they are (not as differences!).

Note that compiled binaries are just artifacts which usefulness (besides milestones, like releases) is very short and the old ones are practically useless (and, if really needed, can be reproduced from older sources).

At the moment, the contents of the repo is ~49MB (checked-out files), but the repo itself is already 222M large(!).

Example of what may happen - at the moment, there are 2679 commits. Assuming that all 4 binaries (linux_x86_64, macosx_aarch64, macosx_x86_64, windows) would be stored and each commit would be followed by a push, this means: push -> build -> commit 10M binaries. Just do the math - the size of the repo would be over 2.5G(!). Even if the binary is committed every 5th commit, it makes 0.5G (what is still rather a lot for a git repo...). With a significant repo size, everything (ie. CI) slows down...

I had an experience with some project where we had to store binaries (~15M per release) in GIT. After couple of years the repo grown to couple of hundreds of MB. The effect was as I described above (operations slowed down).

I would suggest to commit only selected versions of binaries manually (if at all). If automatically - then only tagged and tested versions (like releases). Note that in practice, even in such case, committing binaries to the repo is usually not needed (binary artifacts are usually added separately to the release).

Note also that, for developers, the binaries from the build are stored along with the build job (not in the GIT repo), eg. here:
https://github.com/tebe6502/Mad-Pascal/actions/runs/13496894924
under "artifacts" so are available if needed (probably also some limit can be added on how long they are stored; anyway - they can be deleted, while things in GIT repo - cannot...).

@zbyti
Copy link
Collaborator

zbyti commented Feb 24, 2025

anyway - they can be deleted, while things in GIT repo - cannot...)

Probably we can use git filter-repo

@zbyti
Copy link
Collaborator

zbyti commented Feb 24, 2025

Zrzut ekranu z 2025-02-24 20-26-04
Zrzut ekranu z 2025-02-24 20-35-31
difference after removing history for bin folder - we can do that ;)

python3 git-filter-repo --analyze
python3 git-filter-repo --path bin/ --invert-paths --force

@peterdell
Copy link
Contributor Author

Why did you close without merging?

@zbyti
Copy link
Collaborator

zbyti commented Feb 24, 2025

Sorry, maybe I did it by accident while commenting...

I didn't want to merge because those were changes that had to be accepted by TeBe.

Maybe it happened now while trying to clean up history and reduce the repository by 130 mb?

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.

4 participants